You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@netbeans.apache.org by geertjanw <gi...@git.apache.org> on 2017/10/08 09:37:01 UTC
[GitHub] incubator-netbeans pull request #96: Netbeans 54 review apisupport.refactori...
GitHub user geertjanw opened a pull request:
https://github.com/apache/incubator-netbeans/pull/96
Netbeans 54 review apisupport.refactoring
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/geertjanw/incubator-netbeans netbeans-54-review-apisupport.refactoring
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-netbeans/pull/96.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 #96
----
commit 06da98868310183b783644233c213a2349d689fe
Author: geertjan <ge...@oracle.com>
Date: 2017-10-08T09:18:22Z
[NETBEANS-54] Module Review apisupport.ant
commit b967b4bebcea29b8bc0a0de0703d1801a9659967
Author: geertjan <ge...@oracle.com>
Date: 2017-10-08T09:36:38Z
[NETBEANS-54] Module Review apisupport.refactoring
----
---
[GitHub] incubator-netbeans issue #96: Netbeans 54 review apisupport.refactoring
Posted by geertjanw <gi...@git.apache.org>.
Github user geertjanw commented on the issue:
https://github.com/apache/incubator-netbeans/pull/96
OK, done.
---
[GitHub] incubator-netbeans pull request #96: Netbeans 54 review apisupport.refactori...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-netbeans/pull/96
---
[GitHub] incubator-netbeans pull request #96: Netbeans 54 review apisupport.refactori...
Posted by matthiasblaesing <gi...@git.apache.org>.
Github user matthiasblaesing commented on a diff in the pull request:
https://github.com/apache/incubator-netbeans/pull/96#discussion_r143355629
--- Diff: apisupport.refactoring/test/qa-functional/data/goldenfiles/org/netbeans/modules/apisupport/refactoring/RenameTest/whereUsed.ref ---
@@ -1,3 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
testrename.MyDataLoader
Found 3 occurance(s).
--- End diff --
This is dangerous. And will cause unittests to fail. In this case it causes no problems, because the unittest referencing the file `RenameTest#testWhereUsed` is commented out. In that test a new file is created with some test output and that test output is literally compared to the golden file (`NbTestCase#assertFile`). As there is no IP here, I'd add this file to the build.xml exclude list for rat (the *.pass files are already excluded by a wildcard).
---
[GitHub] incubator-netbeans issue #96: Netbeans 54 review apisupport.refactoring
Posted by geertjanw <gi...@git.apache.org>.
Github user geertjanw commented on the issue:
https://github.com/apache/incubator-netbeans/pull/96
So, specifically for whereUsed.ref, I should remove the license header? Just confirming to make sure I'll be doing the right thing.
---
[GitHub] incubator-netbeans issue #96: Netbeans 54 review apisupport.refactoring
Posted by matthiasblaesing <gi...@git.apache.org>.
Github user matthiasblaesing commented on the issue:
https://github.com/apache/incubator-netbeans/pull/96
Yes I would revert the change to whereUsed.ref and instead add that file to the rat exclusion list. There are other similiar cases already in the build.xml where modifying test-data causes unittests to fail. There might be other files, that fall into the same category.
If I modify unittest data I try to run the unittests and make sure modifying the data does not cause test-failures. If they fail I try to base reasoning on good judgement :-)
---