-
Notifications
You must be signed in to change notification settings - Fork 27
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
GH-369: As a smith I need to bump to Eclipse Oxygen #586
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Please go through the comments below and make sure to test with OPR.
Then 👍 for merge!
@@ -114,10 +118,15 @@ protected static String recursivelyCopyContent(JarURLConnection connection, Stri | |||
} | |||
} else { | |||
checkState(newResource.createNewFile(), "Error while creating new file at: " + newResource); | |||
ByteSink byteSink = Files.asByteSink(newResource); | |||
OutputStream outputStream = byteSink.openStream(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be included in the try-with-resources statement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -226,7 +227,7 @@ public boolean visit(IResource resource) throws CoreException { | |||
return Iterators.unmodifiableIterator(result.iterator()); | |||
} | |||
} | |||
return Iterators.emptyIterator(); | |||
return Iterators.unmodifiableIterator(Collections.emptyIterator()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why add Iterators.unmodifiableIterator(
? Isn't the empty iterator unmodifiable anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. empty iterator is NOT unmodifiable :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
</sources> | ||
<sources> | ||
<source>emf-gen</source> | ||
</sources> | ||
<sources> | ||
<source>grammar-gen</source> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems inconsistent that src-gen
, emf-gen
are removed whereas grammar-gen
isn't removed.
<stream name="master" | ||
label="Master"/> | ||
<stream name="GH-369" | ||
label="GH-369"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't forget to revert this before merging
<repository location="http://download.eclipse.org/egit/updates-4.6"/> | ||
<unit id="ch.qos.logback.classic" version="1.1.2.v20160208-0839"/> | ||
<unit id="com.fasterxml.jackson.core.jackson-databind" version="2.6.2.v20161117-2150"/> | ||
<unit id="com.google.guava" version="18.0.0.v20161115-1643"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why the version of guava changed (from 15.0.0 to 18.0.0)
}; | ||
|
||
CharSource charSrc = byteSource.asCharSource(Charsets.UTF_8); | ||
return charSrc.readLines(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if possible, remove this method entirely and use java.nio.Files#readAllLines()
@@ -9,7 +9,7 @@ Require-Bundle: org.eclipse.n4js, | |||
org.eclipse.n4js.hlc, | |||
org.eclipse.n4js.hlc.base, | |||
org.junit, | |||
com.google.guava;bundle-version="15.0.0", | |||
com.google.guava, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now it's clear!
@@ -1583,6 +1583,11 @@ public static N4JSPackage init() { | |||
|
|||
isInited = true; | |||
|
|||
// Initialize simple dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In few places we explicitly trigger initialization of dependencies (e.g. org.eclipse.n4js.N4JSStandaloneSetup.createInjectorAndDoEMFRegistration()
). With this code being generated now, do we still need to do it? If no
then we should refactor those setup files.
The updated IDE works fine with OPR. |
This PR is close because #637 contains this |
Task: #369
Steps to test
Eclipse IDE for Eclipse committers
as base IDELatest Release (Oxygen)
under Product VersionN4JS -> Oomph Setup URL: https://raw.githubusercontent.com/eclipse/n4js/GH-369/releng/org.eclipse.n4js.targetplatform/N4JS.setup
N4JS-N4 -> Oomph Set URL: https://github.numberfour.eu/raw/NumberFour/n4js-n4/GH-369/releng/com.enfore.n4js.targetplatform/N4JS-N4.setup?token=AAAASzuaHn-xg0r8bto6YlFhkGWktwA_ks5aitjZwA%3D%3D
Still ongoing: