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

Use latest tycho, allow building against kepler, luna, mars #229

Merged
merged 1 commit into from Sep 16, 2016
Merged

Use latest tycho, allow building against kepler, luna, mars #229

merged 1 commit into from Sep 16, 2016

Conversation

mbooth101
Copy link
Contributor

This change updates the build to use the latest version of tycho and allows you to build against any of the last three major Eclipse versions.

By default, it builds against Eclipse Mars. You can build against Eclipse Luna or Kepler by passing, for example: "-Dplatform-version=kepler"

This is useful for making sure eclipse-color-theme remains compatible with all the Eclipse versions that you wish to support.

Kepler by setting the "platform-version" property, for example:

mvn clean verify -Dplatform-version=kepler

License
Copy link
Contributor

Choose a reason for hiding this comment

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

Users just need stable plugins. Having one version that works within 4.3-4.5 is simpler.

Why to have build against Mars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not suggest distributing more than version of the plugin. I merely suggest that you build/test your plugin against all versions of the platform that you intend to support. (This easily allows you to have three CI jobs to run tests against all three supported platforms.)

The point is that the platform may deprecate and remove APIs over time, but if you only ever build/test against the oldest version of the platform, you will not know that your plugin is broken until it is too late when users report it you.

The version of the platform it builds against by default is kinda irrelevant, I can change it to Kepler by default if that is what you'd prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not owner of this project. Build is usually personal preference.

Copy link

@JMTyler JMTyler Aug 24, 2016

Choose a reason for hiding this comment

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

And now these days, it's all too clear why it's important to build/test against all supported versions of the platform.

This plugin is quite simply broken on Neon. I've installed it, and my plugin manager shows that it is installed, but navigating to Window > Preferences > General > Appearance no longer reveals a Color Theme section as it once did on previous versions. It's unusable.

I really wish this PR had been merged a year ago so we wouldn't be in this boat.

Copy link
Member

Choose a reason for hiding this comment

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

That issue is actually a different one (see #241, will be fixed by #244).

Still, this sounds good to me, so I'll merge it soon. Sorry for the delay!

@paulvi
Copy link
Contributor

paulvi commented Aug 29, 2016

@JMTyler raise separate issue with screenshot, more info

@fhd fhd merged commit 963afa3 into eclipse-color-theme:master Sep 16, 2016
@fhd
Copy link
Member

fhd commented Sep 16, 2016

Thanks, merged! Sorry again for the delay. I wonder if we shouldn't make Neon the default target now though.

@mbooth101
Copy link
Contributor Author

@fhd Thanks for merging. Absolutely, changing to Neon by default makes sense these days -- this was a very old PR :-)

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

4 participants