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.

Sign up for GitHub

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

Jump to bottom

review chore: Upgrade JDT version #2052

Merged
merged 8 commits into from
Jun 13, 2018
Merged

review chore: Upgrade JDT version #2052

merged 8 commits into from
Jun 13, 2018

Conversation

surli
Copy link
Collaborator

@surli surli commented Jun 11, 2018

No description provided.

@surli surli changed the title chore: Upgrade JDT version review chore: Upgrade JDT version Jun 11, 2018
@tdurieux
Copy link
Collaborator

This release is more recent: https://mvnrepository.com/artifact/org.eclipse.tycho/org.eclipse.jdt.core/3.14.0.v20171206-0802

@surli
Copy link
Collaborator Author

surli commented Jun 11, 2018

I'm really not sure about that: the tycho's one has been built on december last year, where the one I proposed has been submitted on Maven this year in April.

Tycho and JDT Core don't follow the same release number.

@surli
Copy link
Collaborator Author

surli commented Jun 11, 2018

And I'm sure in mine Java 10 is supported (see #2054 )

@tdurieux
Copy link
Collaborator

Tycho and JDT Core don't follow the same release number.

I'm pretty sure that that the major and minor version number are the same for Tycho and JDT Core

@surli
Copy link
Collaborator Author

surli commented Jun 11, 2018

I just checked locally and apparently Java 10 is not supported in the JDT version you propose.

@tdurieux
Copy link
Collaborator

eclipse...

@surli
Copy link
Collaborator Author

surli commented Jun 11, 2018

So, OK to merge then ? 😛

@monperrus
Copy link
Collaborator

LGTM

tdurieux
tdurieux previously approved these changes Jun 12, 2018
View reviewed changes
src/test/java/spoon/MavenLauncherTest.java Outdated
assertEquals(15, launcher.getEnvironment().getSourceClasspath().length);
// The sourceClasspath seems to change depending on the machine:
// I suspect that the config bring in .m2/settings.xml play a role
// on my machine (Simon U.) and on spoon3r CI machine we got 2 libraries less
Copy link
Collaborator

Choose a reason for hiding this comment

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

#2057 should fix it

@tdurieux tdurieux dismissed their stale review June 12, 2018 09:19

Wrong button :/

@surli
Copy link
Collaborator Author

surli commented Jun 12, 2018

ping @tdurieux @monperrus it's ready

@monperrus
Copy link
Collaborator

LGTM.

Why does a change in JDT version have an impact on getSourceClasspath?

@surli
Copy link
Collaborator Author

surli commented Jun 13, 2018

Why does a change in JDT version have an impact on getSourceClasspath?

Because it brings new transitive dependencies I imagine.

@tdurieux
Copy link
Collaborator

Yes it is the case, I'm still surprise that we double the number of dependencies.

@surli
Copy link
Collaborator Author

surli commented Jun 13, 2018

Just to have an idea, I diff both classpath and here the result:

< /Users/urli/.m2/repository/org/eclipse/tycho/org.eclipse.jdt.core/3.13.50.v20171007-0855/org.eclipse.jdt.core-3.13.50.v20171007-0855.jar
---
> /Users/urli/.m2/repository/org/eclipse/jdt/org.eclipse.jdt.core/3.13.102/org.eclipse.jdt.core-3.13.102.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.core.resources/3.12.0/org.eclipse.core.resources-3.12.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.core.expressions/3.6.0/org.eclipse.core.expressions-3.6.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.core.runtime/3.13.0/org.eclipse.core.runtime-3.13.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.osgi/3.12.100/org.eclipse.osgi-3.12.100.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.equinox.common/3.9.0/org.eclipse.equinox.common-3.9.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.core.jobs/3.9.3/org.eclipse.core.jobs-3.9.3.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.equinox.registry/3.7.0/org.eclipse.equinox.registry-3.7.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.equinox.preferences/3.7.0/org.eclipse.equinox.preferences-3.7.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.core.contenttype/3.6.0/org.eclipse.core.contenttype-3.6.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.equinox.app/1.3.400/org.eclipse.equinox.app-1.3.400.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.core.filesystem/1.7.0/org.eclipse.core.filesystem-1.7.0.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.text/3.6.100/org.eclipse.text-3.6.100.jar
> /Users/urli/.m2/repository/org/eclipse/platform/org.eclipse.core.commands/3.9.0/org.eclipse.core.commands-3.9.0.jar

@monperrus
Copy link
Collaborator

OK, I'll merge

@monperrus monperrus merged commit d1d9778 into INRIA:master Jun 13, 2018
@surli surli mentioned this pull request Jun 25, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdurieux tdurieux tdurieux left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@surli @tdurieux @monperrus