Skip to content

Fix issue #402 missing runtime scope classpath entry#217

Closed
andxu wants to merge 2 commits intomasterfrom
andy_classpath
Closed

Fix issue #402 missing runtime scope classpath entry#217
andxu wants to merge 2 commits intomasterfrom
andy_classpath

Conversation

@andxu
Copy link
Copy Markdown
Contributor

@andxu andxu commented Sep 6, 2018

maven.pomderived=true
test=true
maven.groupId=org.yaml
maven.artifactId=snakeyaml
maven.version=1.19
maven.scope=runtime

maven.pomderived=true
test=true
maven.groupId=org.yaml
maven.artifactId=snakeyaml
maven.version=1.19
maven.scope=runtime
@andxu
Copy link
Copy Markdown
Contributor Author

andxu commented Sep 6, 2018


private static boolean isRuntime(final IClasspathEntry classpathEntry) {
for (IClasspathAttribute attribute : classpathEntry.getExtraAttributes()) {
if (IClasspathManager.SCOPE_ATTRIBUTE.equals(attribute.getName()) && "runtime".equals(attribute.getValue())) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would not add a dependency to m2e.jdt just for that attribute. I suggest you copy the value directly here

for (int j = 0; j < entries.length; j++) {

if (entries[j].getClasspathEntry().isTest()) {
if (entries[j].getClasspathEntry().isTest() && !isRuntime(entries[j].getClasspathEntry())) {
Copy link
Copy Markdown
Collaborator

@fbricon fbricon Sep 6, 2018

Choose a reason for hiding this comment

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

I believe you should always add runtime classes, regardless of their "test" attribute
it's fine

@andxu
Copy link
Copy Markdown
Contributor Author

andxu commented Sep 6, 2018

Moved to the hotfix PR, #219

@andxu andxu closed this Sep 6, 2018
@andxu andxu deleted the andy_classpath branch October 8, 2018 06:31
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.

2 participants