You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@netbeans.apache.org by rosslamont <gi...@git.apache.org> on 2017/10/10 11:24:57 UTC
[GitHub] incubator-netbeans pull request #114: [NETBEANS-54] Module Review css.lib
GitHub user rosslamont opened a pull request:
https://github.com/apache/incubator-netbeans/pull/114
[NETBEANS-54] Module Review css.lib
Added or replaced licenses:
- CssTokenId.java
- NoteType.java
- TokenAcceptorsTest.java
- itabbar.css.testfile
- all css files in unit tests
Still todo:
- antlrv4.patch (not sure what to do here)
- .txt data files (central solution)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rosslamont/incubator-netbeans css_lib
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-netbeans/pull/114.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #114
----
commit c5ba6292be750b3d2c5c771d8c38802324698784
Author: Ross Lamont <rl...@componentcorp.com>
Date: 2017-10-10T11:19:51Z
[NETBEANS-54] Module Review css.lib
Added or replaced licenses:
- CssTokenId.java
- NoteType.java
- TokenAcceptorsTest.java
- itabbar.css.testfile
- all css files in unit tests
Still todo:
- antlrv4.patch
- .txt data files (central solution)
----
---
[GitHub] incubator-netbeans issue #114: [NETBEANS-54] Module Review css.lib
Posted by rosslamont <gi...@git.apache.org>.
Github user rosslamont commented on the issue:
https://github.com/apache/incubator-netbeans/pull/114
My apologies, I assumed tests were part of the default build. Curious: shouldn't travis be running unit tests and picking that up?
I will revert the test changes and look into the rat exclusion.
---
[GitHub] incubator-netbeans issue #114: [NETBEANS-54] Module Review css.lib
Posted by geertjanw <gi...@git.apache.org>.
Github user geertjanw commented on the issue:
https://github.com/apache/incubator-netbeans/pull/114
Looks great and makes sense not to do anything with that patch and .txt data file. Thanks for this review, great.
---
[GitHub] incubator-netbeans issue #114: [NETBEANS-54] Module Review css.lib
Posted by rosslamont <gi...@git.apache.org>.
Github user rosslamont commented on the issue:
https://github.com/apache/incubator-netbeans/pull/114
Sorry - should I also add the patch to the rat exclusions? What would the comment be?
---
[GitHub] incubator-netbeans issue #114: [NETBEANS-54] Module Review css.lib
Posted by matthiasblaesing <gi...@git.apache.org>.
Github user matthiasblaesing commented on the issue:
https://github.com/apache/incubator-netbeans/pull/114
Sorry! I overlooked the patch case. I did not know it yet, but you can add comments to diff patches:
https://stackoverflow.com/questions/18979120/is-it-possible-to-add-a-comment-to-a-diff-file-unified
So I suggest to just add an ALv2 header to the file.
With regard to the squash question: Please do so, this way the atomic change is contained in one commit.
---
[GitHub] incubator-netbeans issue #114: [NETBEANS-54] Module Review css.lib
Posted by matthiasblaesing <gi...@git.apache.org>.
Github user matthiasblaesing commented on the issue:
https://github.com/apache/incubator-netbeans/pull/114
Sorry, but this breaks the unittests. In these the CSS structure is parsed and the result is compared with a reference file. I would do this:
- revert the changes to the files under `css.lib/test/unit/data`
- add an exclusion pattern to the rat report task in the `nbbuild/build.xml` with the pattern `css.lib/test/unit/data/**` with a comment, that these are testdata files
It would be nice if you could have another look at this.
---
[GitHub] incubator-netbeans issue #114: [NETBEANS-54] Module Review css.lib
Posted by rosslamont <gi...@git.apache.org>.
Github user rosslamont commented on the issue:
https://github.com/apache/incubator-netbeans/pull/114
Done.
---