Skip to content
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

Issue 483 #519

Merged
merged 7 commits into from Feb 3, 2020
Merged

Issue 483 #519

merged 7 commits into from Feb 3, 2020

Conversation

fvgh
Copy link
Member

@fvgh fvgh commented Jan 31, 2020

Proposed fix for #483.
The changes remove duplication in the Spotless build scripts.
_ext is build with Gradle 6 like the rest of the Spotless projects.
The _ext projects become sub-projects of the Spotless root project, but are deactivated by default.
The _ext projects are activated by root project properties which allow activation by command line as well as per user gradle.properties.

The changes additionally contain:

  • Work-around of WTP build (latest EFS changes broke the build, fix will be applied with next WTP update). Due to the dependency lock, the current WTP releases are not affected by the EFS change.
  • Fix of JDT tests which are broken since Eclipse 4.13 support #480

@fvgh fvgh requested a review from nedtwigg January 31, 2020 17:26
@fvgh
Copy link
Member Author

fvgh commented Jan 31, 2020

@nedtwigg Please have a look at the mechanism to activate _ext projects. I changed the approach, since I did not wanted to fumble with the Eclipse start-up. I think the user properties is an adequate location to store such user specific modification to the build process.
The p2 conversion still is sometimes buggy (clean is required). But the clean&build for dedicated sub-projects works fine.

Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

This looks excellent to me. It looks to me like there are only two small behavior changes, which I noted as comments below. Neither is a problem, just wanted to call attention to them.

One other small thing is that Goomph had some deprecations in the latest version 3.21.0 - main thing is renaming com.diffplug.gradle.blah to com.diffplug.blah. Might want to incorporate that version bump into this PR.

_ext/eclipse-wtp/gradle.properties Show resolved Hide resolved
_ext/gradle/java-setup.gradle Show resolved Hide resolved
@fvgh
Copy link
Member Author

fvgh commented Feb 1, 2020

@nedtwigg Please have a quick look at last comments and changes. I would prefer a squashed merge, since all changes are just related to build clean-up.

@fvgh fvgh requested a review from nedtwigg February 1, 2020 15:27
@nedtwigg
Copy link
Member

nedtwigg commented Feb 2, 2020

Looks great, I'll let you make a call re: UTF-8 above, then you can squash and merge :)

@fvgh fvgh merged commit c007dba into master Feb 3, 2020
@fvgh fvgh mentioned this pull request Feb 3, 2020
@fvgh fvgh changed the title Issue 483 Build updates Feb 3, 2020
@fvgh fvgh changed the title Build updates Issue 483 Feb 3, 2020
@fvgh
Copy link
Member Author

fvgh commented Feb 3, 2020

@nedtwigg Sorry, was sleeping. Missed that the PR title is used as comment. Think the only way back is an amend and forced push, isn't it?

@nedtwigg
Copy link
Member

nedtwigg commented Feb 3, 2020

Correct, but I think it's fine :)

@fvgh fvgh deleted the issue_483 branch March 11, 2020 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants