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/03/21 18:57:25 UTC
[GitHub] [netbeans] lkishalmi opened a new pull request #3828: Made GradleCommandLine aware of GradleVersion
lkishalmi opened a new pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828
Well, the original motivation for this is that Gradle configuration cache command line argument is only supported from Gradle 6.5 and above. That makes projects before 6.5 non-buildable if the config cache global option is set. So this change makes the GradleCommandLine aware of GradleVersion and removes options which are not yet or no longer supported.
@JaroslavTulach Is it Ok to expose a third party class GradleVersion that much throughout an API?
This is a working draft. I'll add additional, tests, logging and documentation.
--
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] lkishalmi commented on a change in pull request #3828: Made GradleCommandLine aware of GradleVersion
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828#discussion_r832304265
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/api/execute/GradleCommandLine.java
##########
@@ -88,58 +89,82 @@
* @since 2.23
*/
public interface GradleOptionItem {
+ /**
+ * Shall return {@code true} if the IDE supports this option item.
+ *
+ * @return {@code true} if this option is supported by the IDE
+ */
boolean isSupported();
+ /**
+ * Shall return {@code true} if this option is supported by the provided
+ * GradleVersion.
+ *
+ * @param ver the GradleVersion to check.
+ * @return {@code true} if the provided Gradle version supports this option.
+ */
Review comment:
There will be an updated commit with javadoc and tests and arch changes. Let's just check the code itself 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] lkishalmi commented on a change in pull request #3828: Made GradleCommandLine aware of GradleVersion
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828#discussion_r832305730
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/loaders/ModelCache.java
##########
@@ -100,4 +109,12 @@ private void load() {
public synchronized State getState() {
return state;
}
+
+ private GradleVersion compatibleGradleVersion() {
Review comment:
Could find a place for it somewhere...
--
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] lkishalmi commented on a change in pull request #3828: Made GradleCommandLine aware of GradleVersion
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828#discussion_r832309740
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/loaders/LegacyProjectLoader.java
##########
@@ -194,7 +199,15 @@ private static GradleProject loadGradleProject(ReloadContext ctx, CancellationTo
}
return ret;
}
-
+
+ private static GradleVersion compatibleGradleVersion(Project prj) {
Review comment:
Oh, good catch, I left the checking of the IDE Java Version out from this computation!
--
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] sdedic commented on pull request #3828: Made GradleCommandLine aware of GradleVersion
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828#issuecomment-1076004245
> do you see value in this PR for the future?
I personally like the version meta-data for options / parameters; especially the user action mapping (custom actions) may eventually go out of sync. Can be used to report project problems. Code completion can also offer only relevant items in action editors.
The only debatable issue is the leakage of `GradleVersion` from Gradle libs outside of Gradle Projects API to its clients -- @jtulach should have an option on 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.
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] sdedic commented on a change in pull request #3828: Made GradleCommandLine aware of GradleVersion
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828#discussion_r831942423
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/api/execute/GradleVersionRange.java
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ */
+package org.netbeans.modules.gradle.api.execute;
+
+import org.gradle.util.GradleVersion;
+
+/**
+ *
Review comment:
Javadoc + `@since`
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/loaders/LegacyProjectLoader.java
##########
@@ -194,7 +199,15 @@ private static GradleProject loadGradleProject(ReloadContext ctx, CancellationTo
}
return ret;
}
-
+
+ private static GradleVersion compatibleGradleVersion(Project prj) {
Review comment:
Nitpick: Does this return a gradle version **compatible** with the current project, or the **configured** one ? I mean if the project does not specify anything, the current default version is returned regardless of project's structure
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/loaders/ModelCache.java
##########
@@ -100,4 +109,12 @@ private void load() {
public synchronized State getState() {
return state;
}
+
+ private GradleVersion compatibleGradleVersion() {
Review comment:
Duplicate method - could it be moved to some module-private Util class ?
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/api/execute/GradleCommandLine.java
##########
@@ -88,58 +89,82 @@
* @since 2.23
*/
public interface GradleOptionItem {
+ /**
+ * Shall return {@code true} if the IDE supports this option item.
+ *
+ * @return {@code true} if this option is supported by the IDE
+ */
boolean isSupported();
+ /**
+ * Shall return {@code true} if this option is supported by the provided
+ * GradleVersion.
+ *
+ * @param ver the GradleVersion to check.
+ * @return {@code true} if the provided Gradle version supports this option.
+ */
Review comment:
`@since`
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/api/execute/GradleCommandLine.java
##########
@@ -525,15 +596,22 @@ public ParametricArgument parse(String arg, Iterator<String> args) {
}
//</editor-fold>
+ final GradleVersion gradleVersion;
final Set<Argument> arguments = new LinkedHashSet<>();
final Set<String> tasks = new LinkedHashSet<>();
- public GradleCommandLine(GradleCommandLine cmd) {
+ public GradleCommandLine(GradleVersion gradleVersion, GradleCommandLine cmd) {
Review comment:
Javadoc + `@since`
--
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] lkishalmi commented on pull request #3828: Made GradleCommandLine aware of GradleVersion
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828#issuecomment-1075330631
A note on why I considered to filter existing command line options late instead of filter them before. I usually think of a Gradle command line as a space separated string stored somewhere. When I pass that command line to the GradleCommandLine, I'd expect the whole command line to be kept there at least in semantics. Then it can be copied, modified and filtered with the provided methods.
However you've got me thinking. As it all started with ```--configuration-cache``` could be globally enabled. That flag is used maybe 3 times in the code and can be checked the GradleVersion on those spots to add that flag or clear that. So for that problem this implementation is an overkill. I've got carried away as the GradleCommandLine has already have a getSupportedCommandLine() method and I thought it would be fun, if that would understand the target GradleVersion as well.
@sdedic @JaroslavTulach do you see value in this PR for the future?
--
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] lkishalmi commented on pull request #3828: Made GradleCommandLine aware of GradleVersion
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on pull request #3828:
URL: https://github.com/apache/netbeans/pull/3828#issuecomment-1076734438
OFFTOPIC: I wish I could have that debate with you guys next to a beer somewhere in Prague. It's been 20 almost twenty years since I last visited there.
I was thinking about use GradleDistribution instead of GradleVersion in the API, and the GradleVersionRange can be moved out of the API package to somewhere else...
--
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