Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[NETBEANS-54] Module Review editor #42

Closed
wants to merge 3 commits into from
Closed

[NETBEANS-54] Module Review editor #42

wants to merge 3 commits into from

Conversation

vieiro
Copy link
Contributor

@vieiro vieiro commented Oct 3, 2017

  • No external libraries.
  • Checked Rat report: added license to project files. Did NOT add license to editor/demosrc/README nor to the rest of test files.
  • Unit tests org.netbeans.editor.PlainDocumentCompatibilityRandomTest and org.netbeans.modules.editor.NbEditorToolBarTest fail (but they were also failing before this PR).
  • QA Tests should be run.

@vieiro
[NETBEANS-54] Module Review editor
6f7d84f 
- No external libraries.
- Checked Rat report: added license to project files. Did NOT add license to editor/demosrc/README nor to the rest of test files.
- Unit tests org.netbeans.editor.PlainDocumentCompatibilityRandomTest and org.netbeans.modules.editor.NbEditorToolBarTest fail (but they were also failing before this PR).
- QA Tests should be run.
@junichi11
Copy link
Member

Merged in 215acff.

@vieiro Do you mind closing this PR?

@vieiro
Copy link
Contributor Author

vieiro commented Oct 7, 2017

Closing PR as requested.

@vieiro vieiro closed this Oct 7, 2017
@geertjanw
Copy link
Member

What about these, can you exclude them via Rat or license them:

editor/demosrc/base/org/netbeans/editor/example/res/template.html_
editor/demosrc/base/org/netbeans/editor/example/res/template.java_
editor/demosrc/properties-addon/org/netbeans/editor/example/res/template.properties
editor/test/qa-functional/src/org/netbeans/test/editor/suites/keybindings/actions.txt
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation1.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation2.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation3.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation4.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation5.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation6.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation7.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation8.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotationBrowseable1.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotationBrowseable2.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotationBrowseable3.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotationPriority1.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotationPriority2.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotationPriority3.xml
editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotationPriority4.xml

@geertjanw
Copy link
Member

Or is there a problem when these files are relicensed, e.g., tests fail?

@geertjanw
Copy link
Member

Reopening with the above question.

@geertjanw geertjanw reopened this Nov 2, 2017
@vieiro
Copy link
Contributor Author

vieiro commented Nov 2, 2017

Will take a look at them tonight.

junichi11
junichi11 reviewed Nov 3, 2017
View reviewed changes
Copy link
Member

@junichi11 junichi11 left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please create a new branch and a new PR because this branch has the merged commit?

nbbuild/build.xml
@@ -2002,6 +2002,14 @@ It is possible to use -Ddebug.port=3234 -Ddebug.pause=y to start the system in d
<exclude name="debugger.jpda/test/unit/src/org/netbeans/api/debugger/jpda/testapps/JspLineBreakpointApp.txt" /> <!-- test data -->
<exclude name="diff/test/unit/src/org/netbeans/modules/diff/builtin/provider/*.txt" /> <!--test data-->
<exclude name="diff/test/unit/src/org/netbeans/modules/diff/builtin/visualizer/data/**" /> <!--test data-->
<exclude name="editor/test/qa-functional/data/projects/editor_test/src/**.txt" /> <!-- test data -->
<exclude name="editor/test/qa-functional/data/**.pass" /> <!-- qa test data -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines are excluded with <exclude name="*/test/qa-functional/data/**" />.

nbbuild/build.xml
<exclude name="editor/test/qa-functional/data/projects/editor_test/src/**.txt" /> <!-- test data -->
<exclude name="editor/test/qa-functional/data/**.pass" /> <!-- qa test data -->
<exclude name="editor/test/qa-functional/src/org/netbeans/test/editor/suites/keybindings/actions.txt" /> <!-- qa test data -->
<exclude name="editor/test/qa-functional/data/projects/editor_test/src/org/netbeans/test/editor/search/IncrementalSearchTest/**.txt" /> <!-- qa test data -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is excluded with <exclude name="*/test/qa-functional/data/**" />.

nbbuild/build.xml
<exclude name="editor/test/qa-functional/data/**.pass" /> <!-- qa test data -->
<exclude name="editor/test/qa-functional/src/org/netbeans/test/editor/suites/keybindings/actions.txt" /> <!-- qa test data -->
<exclude name="editor/test/qa-functional/data/projects/editor_test/src/org/netbeans/test/editor/search/IncrementalSearchTest/**.txt" /> <!-- qa test data -->
<exclude name="editor/test/unit/src/org/netbeans/modules/editor/resources/**.xml" /> <!-- unit test data -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest <exclude name="editor/test/unit/src/org/netbeans/modules/editor/resources/testAnnotation*.xml" />.

@vieiro
Copy link
Contributor Author

vieiro commented Nov 3, 2017

Thanks, guys, for your reviews.

I'll do a new PR tonight/EU Time.

@vieiro
Copy link
Contributor Author

vieiro commented Nov 4, 2017

See #236 #236

@geertjanw I missed your first comments completely (lost in email folders), please open an issue for these updates, I won't miss those!

Closing this PR now.

@vieiro vieiro closed this Nov 4, 2017
@vieiro vieiro deleted the netbeans-54-module-review-editor branch December 16, 2017 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@junichi11 junichi11 junichi11 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@vieiro @junichi11 @geertjanw