You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/01/14 23:35:05 UTC

[GitHub] [netbeans] BradWalker opened a new pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

BradWalker opened a new pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872
 
 
   …ence
   
   It's really not proper to use a static variable through a reference because they can't be derived. The proper thing to do is use them with a Class name..
   
      [repeat] /home/bwalker/src/netbeans/java/java.hints.ui/src/org/netbeans/modules/java/hints/spiimpl/refactoring/ConfigurationsComboModel.java:175: warning: [static] static variable should be qualified by type name, KeyEvent, instead of by an expression
      [repeat]             if (ke.getKeyCode() == ke.VK_ENTER || ke.getKeyCode() == ke.VK_ESCAPE) {
      [repeat]                                      ^
      [repeat] /home/bwalker/src/netbeans/java/java.hints.ui/src/org/netbeans/modules/java/hints/spiimpl/refactoring/ConfigurationsComboModel.java:175: warning: [static] static variable should be qualified by type name, KeyEvent, instead of by an expression
      [repeat]             if (ke.getKeyCode() == ke.VK_ENTER || ke.getKeyCode() == ke.VK_ESCAPE) {
      [repeat]
   
   This change fixes that..

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] eirikbakke commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
eirikbakke commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575255481
 
 
   Yes, I think cleaning up the build output is an honorable cause. It's just more work than it seems, because of the need to test every piece of code that is changed. For trivial changes you can sometimes just stare at the diff and think "what could possibly go wrong?"--and yet, somtimes it does.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#discussion_r367040956
 
 

 ##########
 File path: ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java
 ##########
 @@ -119,10 +119,10 @@ public LinkButton() {
     }
     
     public void setColors(Color linkColor, Color linkInFocusColor, Color mouseOverLinkColor, Color visitedLinkColor) {
-        this.linkInFocusColor = linkInFocusColor;
-        this.linkColor = linkColor;
-        this.mouseOverLinkColor = mouseOverLinkColor;
-        this.visitedLinkColor = visitedLinkColor;
+        linkInFocusColor = linkInFocusColor;
+        linkColor = linkColor;
+        mouseOverLinkColor = mouseOverLinkColor;
+        visitedLinkColor   = visitedLinkColor;
 
 Review comment:
   No problem. It's now changed.. Thanks for the feedback..

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] eirikbakke commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
eirikbakke commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575249171
 
 
   This change touches a lot of different modules--have you tested each affected feature to make sure no problems are introduced?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker edited a comment on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker edited a comment on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575251997
 
 
   I have used the changes as part of my day-to-day use of Netbeans. It works correctly and the C/C++ module continued to run with no problem.  Most of the changes are pretty benign: instead of referencing a class static field through a reference we use the properly qualified type..
   
   For example (from a diff),
   
   ```
               - if (next.getNodeType() == **next**.DOCUMENT_TYPE_NODE) {
               + if (next.getNodeType() == **Node**.DOCUMENT_TYPE_NODE) {
   ```
   
   Notice how I'm only referencing the class field.. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker edited a comment on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker edited a comment on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575252947
 
 
   I mostly agree with your comments..  My focus is on trying to get a "clean" build with a very minimal amount of warnings.. 
   
   BTW, thanks for the feedback!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mklaehn commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
mklaehn commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#discussion_r366634412
 
 

 ##########
 File path: ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java
 ##########
 @@ -119,10 +119,10 @@ public LinkButton() {
     }
     
     public void setColors(Color linkColor, Color linkInFocusColor, Color mouseOverLinkColor, Color visitedLinkColor) {
-        this.linkInFocusColor = linkInFocusColor;
-        this.linkColor = linkColor;
-        this.mouseOverLinkColor = mouseOverLinkColor;
-        this.visitedLinkColor = visitedLinkColor;
+        linkInFocusColor = linkInFocusColor;
+        linkColor = linkColor;
+        mouseOverLinkColor = mouseOverLinkColor;
+        visitedLinkColor   = visitedLinkColor;
 
 Review comment:
   Seems wrong and unrelated to the described issues

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mklaehn commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
mklaehn commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#discussion_r366635901
 
 

 ##########
 File path: ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java
 ##########
 @@ -119,10 +119,10 @@ public LinkButton() {
     }
     
     public void setColors(Color linkColor, Color linkInFocusColor, Color mouseOverLinkColor, Color visitedLinkColor) {
-        this.linkInFocusColor = linkInFocusColor;
-        this.linkColor = linkColor;
-        this.mouseOverLinkColor = mouseOverLinkColor;
-        this.visitedLinkColor = visitedLinkColor;
+        linkInFocusColor = linkInFocusColor;
+        linkColor = linkColor;
+        mouseOverLinkColor = mouseOverLinkColor;
+        visitedLinkColor   = visitedLinkColor;
 
 Review comment:
   Well, maybe not unrelated but wrong.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mklaehn commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
mklaehn commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#discussion_r367028572
 
 

 ##########
 File path: ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java
 ##########
 @@ -119,10 +119,10 @@ public LinkButton() {
     }
     
     public void setColors(Color linkColor, Color linkInFocusColor, Color mouseOverLinkColor, Color visitedLinkColor) {
-        this.linkInFocusColor = linkInFocusColor;
-        this.linkColor = linkColor;
-        this.mouseOverLinkColor = mouseOverLinkColor;
-        this.visitedLinkColor = visitedLinkColor;
+        linkInFocusColor = linkInFocusColor;
+        linkColor = linkColor;
+        mouseOverLinkColor = mouseOverLinkColor;
+        visitedLinkColor   = visitedLinkColor;
 
 Review comment:
   Short answer something like `LinkButton.linkInFocusColor = linkInFocusColor` for these four.
   
   Long answer: If you simply remove the `this.` you essentially assign the method parameter to itself. Highlighting in NetBeans Editor will show that as well.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575252947
 
 
   I mostly agree with your comments..  My focus is on trying to get a "clean" build with a very minimal amount of warnings.. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] eirikbakke merged pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
eirikbakke merged pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575266069
 
 
   Agreed.. That's the risk we take otherwise this code will never get cleaned up.. Also, I've noticed a LOT of places where there are "bugs" as a result of me just looking at the code when I make the changes.
   
   Thanks for merging this!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker commented on a change in pull request #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#discussion_r367025507
 
 

 ##########
 File path: ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java
 ##########
 @@ -119,10 +119,10 @@ public LinkButton() {
     }
     
     public void setColors(Color linkColor, Color linkInFocusColor, Color mouseOverLinkColor, Color visitedLinkColor) {
-        this.linkInFocusColor = linkInFocusColor;
-        this.linkColor = linkColor;
-        this.mouseOverLinkColor = mouseOverLinkColor;
-        this.visitedLinkColor = visitedLinkColor;
+        linkInFocusColor = linkInFocusColor;
+        linkColor = linkColor;
+        mouseOverLinkColor = mouseOverLinkColor;
+        visitedLinkColor   = visitedLinkColor;
 
 Review comment:
   In it's original form, here is the warning emitted..
   ```
      [repeat] /home/bwalker/src/netbeans/ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java:122: warning: [static] static variable should be qualified by type name, LinkButton, instead of by an expression
      [repeat]         this.linkInFocusColor = linkInFocusColor;
      [repeat]             ^
      [repeat] /home/bwalker/src/netbeans/ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java:123: warning: [static] static variable should be qualified by type name, LinkButton, instead of by an expression
      [repeat]         this.linkColor = linkColor;
      [repeat]             ^
      [repeat] /home/bwalker/src/netbeans/ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java:124: warning: [static] static variable should be qualified by type name, LinkButton, instead of by an expression
      [repeat]         this.mouseOverLinkColor = mouseOverLinkColor;
      [repeat]             ^
      [repeat] /home/bwalker/src/netbeans/ide/team.commons/src/org/netbeans/modules/bugtracking/commons/LinkButton.java:125: warning: [static] static variable should be qualified by type name, LinkButton, instead of by an expression
      [repeat]         this.visitedLinkColor   = visitedLinkColor;
      [repeat]             ^
   ```
   Which seems correct as these variables are declared static in the class. So by accessing them via the self reference is not correct and the compiler is warning us. Nor is providing a fully qualified class name the correct way to fix this. These variables are declared as static in the class so they have "global" reference.
   
   What would you like for me to change it to?
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker edited a comment on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker edited a comment on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575251997
 
 
   I have used the changes as part of my day-to-day use of Netbeans. It works correctly and the C/C++ module continued to run with no problem.  Most of the changes are pretty benign: instead of referencing a class static field through a reference we use the properly qualified type..
   
   For example (from a diff),
   
   ```
               - if (next.getNodeType() == next.DOCUMENT_TYPE_NODE) {
               + if (next.getNodeType() == Node.DOCUMENT_TYPE_NODE) {
   ```
   
   Notice how I'm only referencing the class field.. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
BradWalker commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575251997
 
 
   I have used the changes as part of my day-to-day use of Netbeans. It works correctly and the C/C++ module continued to run with no problem.  Most of the changes are pretty benign: instead of referencing a class static field through a reference we use the properly qualified type..
   
   For example,
   
   ```
               if (next.getNodeType() == **next**.DOCUMENT_TYPE_NODE) {
               if (next.getNodeType() == **Node**.DOCUMENT_TYPE_NODE) {
   ```
   
   Notice how I'm only referencing the class field.. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] eirikbakke commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…

Posted by GitBox <gi...@apache.org>.
eirikbakke commented on issue #1872: [NETBEANS-3695] - not proper to use a static variable through a refer…
URL: https://github.com/apache/netbeans/pull/1872#issuecomment-575252128
 
 
   OK, never mind, the changes seem safe. Though in general, it's best to do these kinds changes only once you're actually working on, and testing, a given piece of code.
   
   It's very easy to introduce errors (as with LinkButton), and code should assumed to be "broken unless tested".

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists