Skip to content

[NETBEANS-3041] Add gradle support org.gradle.jvmargs #1425

Closed
one-leaf wants to merge 3 commits intoapache:masterfrom
one-leaf:master
Closed

[NETBEANS-3041] Add gradle support org.gradle.jvmargs #1425
one-leaf wants to merge 3 commits intoapache:masterfrom
one-leaf:master

Conversation

@one-leaf
Copy link
Copy Markdown

add read jvmargs from gradle.properties.

@junichi11 junichi11 requested a review from lkishalmi August 14, 2019 10:45
@junichi11 junichi11 added the Gradle [ci] enable "build tools" tests label Aug 14, 2019
@junichi11
Copy link
Copy Markdown
Member

@one-leaf Did you read this? https://netbeans.apache.org/participate/submit-pr.html
You should create a branch.
Did you read @lkishalmi 's comments? #1416 (review)

@one-leaf
Copy link
Copy Markdown
Author

@one-leaf Did you read this? https://netbeans.apache.org/participate/submit-pr.html
You should create a branch.
Did you read @lkishalmi 's comments? #1416 (review)

Yes. I've read it.

@junichi11
Copy link
Copy Markdown
Member

@one-leaf Why didn't you create a new ticket[1] and a branch??
[1] https://issues.apache.org/jira/browse/NETBEANS

Copy link
Copy Markdown
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Also do not forget to increment the implementation version.
The JIRA issue is still required, and this PR shall be labelled as API Change.
Other than that, it is a nice piece of work. Thanks!

}
projectDir = dir;
rootDir = projectDir;
searchPropertyFiles();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably it would be better to calculate the property files whenever the getPropertFiles() call is made, also it shall it shall not be stored as a member variable. It also shall return an empty array if the GradleFiles does not represent a project folder. Also the returned list shall be immutable.
It seems I've totally forgotten to implement that method. Thanks for doing that.

}
}

public void configure(ConfigurableLauncher launcher, File projectDir) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is API change, shall be documented with at least a @since

@lkishalmi lkishalmi added the API Change [ci] enable extra API related tests label Aug 21, 2019
@lkishalmi lkishalmi changed the title add gradle support org.gradle.jvmargs [NETBEANS-3041] Add gradle support org.gradle.jvmargs Sep 3, 2019
@lkishalmi
Copy link
Copy Markdown
Contributor

@one-leaf In the meantime this issue has been reported as (NETBEANS-3041)[https://issues.apache.org/jira/browse/NETBEANS-3041] So do not worry about that. Are you able to make the requested changes or shall I take over this PR?

@lkishalmi
Copy link
Copy Markdown
Contributor

Closing in favor of #1501

@lkishalmi lkishalmi closed this Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API Change [ci] enable extra API related tests Gradle [ci] enable "build tools" tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants