Skip to content

Ensure that the -parameters compiler flag is included in boot applications built with Gradle #9839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

kashike
Copy link
Contributor

@kashike kashike commented Jul 22, 2017

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 22, 2017
@philwebb
Copy link
Member

Thanks for the pull-request! I'm wondering if always applying the -parameters flag is the right call. The problem that I can foresee is that someone specifically doesn't want -parameters and they now have no way to achieve that.

The configureUtf8Encoding method was quite nice because if the user did set an encode it would back off. I don't see an easy way to do this with -parameters.

@wilkinsona any suggestions?

@wilkinsona
Copy link
Member

There are a couple of things we could do:

  1. Set it up front (rather than deferring it to afterEvaluate) and allow the user to override it by setting the args back to an empty list.
  2. Set it in afterEvaluate but only if the compiler args are empty

@kashike
Copy link
Contributor Author

kashike commented Jul 24, 2017

Let me know if the changes I've made are what you're looking for.

@wilkinsona
Copy link
Member

@kashike Thanks for the update. Sorry, looks like I've created some confusion. My two suggestions were intended as two different options. We need to decide which one we prefer and just implement that one.

I think I'm leaning towards 1 as it's quite flexible:

  1. You get -parameters by default
  2. It's easy to add additional args
  3. You can override it by setting an empty list or a list that doesn't include -parameters

After more thought, the second suggestion may not work. If we set -parameters when the list is empty I'm not sure how you'd indicate that you don't want it. Perhaps with null but I'm not sure I like that or if it would even be permitted.

In summary, the first option is the best I can come up with. It'd be great to have some tests that cover the three scenarios I've listed above showing that default, additional args, and overridden args all work.

We should also update the docs to mention that -parameters is set as one of the reactions to the Java plugin being applied. That's something we can take care of as part of the merge, though, so please don't feel obliged to do it.

@kashike
Copy link
Contributor Author

kashike commented Jul 24, 2017

No problem, @wilkinsona. I've made some changes, which I think are what you've suggested - let me know.

@philwebb philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 26, 2017
@philwebb philwebb marked this as a duplicate of #9831 Jul 26, 2017
@philwebb philwebb added this to the 2.0.0.M5 milestone Jul 26, 2017
@wilkinsona wilkinsona self-assigned this Sep 20, 2017
wilkinsona added a commit that referenced this pull request Sep 20, 2017
* gh-9839:
  Polish "Use -parameters compiler arg by default in Gradle builds"
  Use -parameters compiler arg by default in Gradle builds
@wilkinsona
Copy link
Member

@kashike Thank you for making your first contribution to Spring Boot. I've merged this into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants