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 2022/02/25 15:52:36 UTC
[GitHub] [netbeans] mklaehn opened a new pull request #3676: Update project platform/api.search to JDK8 source level and remove wa…
mklaehn opened a new pull request #3676:
URL: https://github.com/apache/netbeans/pull/3676
…rnings.
- add override annotations
- using multi-catch
- using try-with-resources
- using diamond operator
- use JDK 5 for-loop
- convert to lambda or member reference
- handle deprecations where possible
---
**^Add meaningful description above**
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
- are all your own work, and you have the right to contribute them.
- are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).
Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field.
If you're a first time contributor, see the Contributing guidelines for more information.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #3676: Update project platform/api.search to JDK8 source level and remove wa…
Posted by GitBox <gi...@apache.org>.
mklaehn commented on a change in pull request #3676:
URL: https://github.com/apache/netbeans/pull/3676#discussion_r815298291
##########
File path: platform/api.search/src/org/netbeans/api/search/provider/SearchInfo.java
##########
@@ -125,15 +125,8 @@
@NonNull final SearchScopeOptions options,
@NonNull final SearchListener listener,
@NonNull final AtomicBoolean terminated) {
-
- return new Iterable<FileObject>() {
-
- @Override
- public Iterator<FileObject> iterator() {
- return createFilesToSearchIterator(options, listener,
- terminated);
- }
- };
+ return () -> createFilesToSearchIterator(options, listener,
+ terminated);
Review comment:
I think this practices is from the time when code had to be pressed into 80 character width lines. Something more lines in this class adhere to as well.
Anyway done as suggested (avoid a complete reformatting ;)).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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] mbien commented on a change in pull request #3676: Update project platform/api.search to JDK8 source level and remove wa…
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3676:
URL: https://github.com/apache/netbeans/pull/3676#discussion_r815247634
##########
File path: platform/api.search/src/org/netbeans/api/search/provider/SearchInfo.java
##########
@@ -125,15 +125,8 @@
@NonNull final SearchScopeOptions options,
@NonNull final SearchListener listener,
@NonNull final AtomicBoolean terminated) {
-
- return new Iterable<FileObject>() {
-
- @Override
- public Iterator<FileObject> iterator() {
- return createFilesToSearchIterator(options, listener,
- terminated);
- }
- };
+ return () -> createFilesToSearchIterator(options, listener,
+ terminated);
Review comment:
This is just a nitpick but, since modern java is quite a bit more compact, I think you could remove the line break here and other places (esp if there is just a single word and a closing bracket on the new line).
I am not suggesting to format everything (please don't :)), just the worst offenders in the areas you changed and which became more compact.
##########
File path: platform/api.search/src/org/netbeans/modules/search/ui/ResultsOutlineCellRenderer.java
##########
@@ -58,24 +58,29 @@ public Component getTableCellRendererComponent(JTable table, Object value,
Property<?> property = (Property<?>) value;
try {
String valueString = getDisplayValue(property);
- if (property.getName().equals("path")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- computeFitText(table, row, column, valueString),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else if (property.getName().equals("size")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- formatFileSize((Long) property.getValue()),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else if (property.getName().equals("lastModified")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- formatDate((Date) property.getValue()),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else {
- renderer = super.getTableCellRendererComponent(table,
+ switch (property.getName()) {
Review comment:
this is potentially dangerous since `name` could be null, and switch doesn't like null.
We could keep the old if-else chain here, until we rise the language level far enough that we can use the new switch syntax, which tolerates null. There is not much gained readability wise and this also doesn't look like performance sensitive code, so i think we can live with if-else here (for now).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 merged pull request #3676: Update project platform/api.search to JDK8 source level and remove wa…
Posted by GitBox <gi...@apache.org>.
mklaehn merged pull request #3676:
URL: https://github.com/apache/netbeans/pull/3676
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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 #3676: Update project platform/api.search to JDK8 source level and remove wa…
Posted by GitBox <gi...@apache.org>.
mklaehn commented on a change in pull request #3676:
URL: https://github.com/apache/netbeans/pull/3676#discussion_r815298848
##########
File path: platform/api.search/src/org/netbeans/modules/search/ui/ResultsOutlineCellRenderer.java
##########
@@ -58,24 +58,29 @@ public Component getTableCellRendererComponent(JTable table, Object value,
Property<?> property = (Property<?>) value;
try {
String valueString = getDisplayValue(property);
- if (property.getName().equals("path")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- computeFitText(table, row, column, valueString),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else if (property.getName().equals("size")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- formatFileSize((Long) property.getValue()),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else if (property.getName().equals("lastModified")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- formatDate((Date) property.getValue()),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else {
- renderer = super.getTableCellRendererComponent(table,
+ switch (property.getName()) {
Review comment:
If you're suggesting that `property.getName()` may be null, a value that by convention shall not be null, is preset to an empty String and is the default value for `FeatureDescriptor.getDisplayName()` and `FeatureDescriptor.getShortDescription()` should they be unset and blindly used in `Property.equals(Object)` then something is horrificably 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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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] mbien commented on a change in pull request #3676: Update project platform/api.search to JDK8 source level and remove wa…
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3676:
URL: https://github.com/apache/netbeans/pull/3676#discussion_r815309179
##########
File path: platform/api.search/src/org/netbeans/modules/search/ui/ResultsOutlineCellRenderer.java
##########
@@ -58,24 +58,29 @@ public Component getTableCellRendererComponent(JTable table, Object value,
Property<?> property = (Property<?>) value;
try {
String valueString = getDisplayValue(property);
- if (property.getName().equals("path")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- computeFitText(table, row, column, valueString),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else if (property.getName().equals("size")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- formatFileSize((Long) property.getValue()),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else if (property.getName().equals("lastModified")) { //NOI18N
- renderer = super.getTableCellRendererComponent(table,
- formatDate((Date) property.getValue()),
- isSelected, hasFocus, row, column);
- setToolTip(renderer, property);
- } else {
- renderer = super.getTableCellRendererComponent(table,
+ switch (property.getName()) {
Review comment:
i haven't looked at all paths, but I did in fact see that the constructor sets it to "" by default - which was promising - but there is also a setter in `FeatureDescriptor` without a null check, So it appears null is a tolerated value.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
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