Skip to content
This repository has been archived by the owner on Nov 10, 2017. It is now read-only.

Use mutable classpath api #354

Merged
merged 2 commits into from
Jul 6, 2015

Conversation

WonderCsabo
Copy link
Contributor

Related to #353.

It now uses the mutable IClasspathEntryDescriptor API instead of the
IClasspathEntry API.
@WonderCsabo
Copy link
Contributor Author

I do not know why Travis is still failing. I can confirm that locally my change fixes the issue, and without my change the issue consistently appears.

@WonderCsabo
Copy link
Contributor Author

OK, i removed the FixerWorkspaceConfigurer, and the problem is still here. @rgladwell if you have no other ideas, we may have to request a debug VM from Travis to actually debug this thing...

@WonderCsabo
Copy link
Contributor Author

I also removed the call to JavaProjectConfigurator, and the problem still persists.

@rgladwell
Copy link
Owner

I can reproduce the issue you're seeing from the command line Maven build on my local machine.

@WonderCsabo
Copy link
Contributor Author

Maybe #344 is related. Let's wait for @fbricon's answer there.

@WonderCsabo
Copy link
Contributor Author

@rgladwell apart from technical advantages, this may (partially) solve #245.

This is the order which is created for my project:
untitled

Of course it does not replaces a complete sorter, for example if you delete src/main/resources, gen will be first. But if you do not delete anything, the order will be good with this PR.

I think it can be merged, this is a valuable PR for multiple reasons.

@rgladwell
Copy link
Owner

So this PR alone does not actually fix anything yet? We need fixes for #359 and #360 as well?

@rgladwell
Copy link
Owner

Sorry that last comment sounded rude, I meant to say "this PR needs other code changes to fix #353?"

@WonderCsabo
Copy link
Contributor Author

This PR is a spinoff. I thought it will fix the problem, but it doesn't and it cannot. However this is still contains valuable changes, as i already tried to explain. This is not related to m2e 1.6, that was only my false assumption. I will update the misleading second commit message, sorry for not doing that before.

They are now using the mutable IClassPathEntryDescriptor API, and
directly mutating the existing entries instead of removing and adding
new ones.
@WonderCsabo
Copy link
Contributor Author

PR (commit message) updated.

@rgladwell
Copy link
Owner

So I understand this is unrelated to #353, and this could be a fix for #245? Should we pull in the tests from #248 to verify this?

@WonderCsabo
Copy link
Contributor Author

I do not think so. This is just a refactor PR. It does not really fix that problem, only a subset of that, and only as a side effect. Let's forget #245 here.

@rgladwell
Copy link
Owner

OK

rgladwell added a commit that referenced this pull request Jul 6, 2015
@rgladwell rgladwell merged commit dfe124a into rgladwell:master Jul 6, 2015
@WonderCsabo WonderCsabo deleted the useMutableClasspathAPI branch July 6, 2015 15:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants