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

refactor: Excluding decompiler related features from spoon-core #2766

Merged
merged 23 commits into from
Nov 26, 2018

Conversation

nharrand
Copy link
Collaborator

Attempt to implement #2747
Related to #2737

@nharrand nharrand changed the title WiP: refactor: Excluding decopiler related feature from spoon-core WiP: refactor: Excluding decompiler related features from spoon-core Nov 15, 2018
@nharrand
Copy link
Collaborator Author

nharrand commented Nov 15, 2018

This is a PoC of what could be a split of spoon into different artifacts. WDYT?
I would like to reuse architectural tests (unused import, package creation, force documentation of public api...), does anyone have an opinion on how to make this tests more reusable?

@monperrus
Copy link
Collaborator

Looks really good to me. This will enable to have a clean Maven central release without any odd dependency.

We can also add a travis job to test it.

@surli with respect to our conversation, WDYT?

spoon-bytecode/pom.xml Outdated Show resolved Hide resolved
spoon-bytecode/pom.xml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
doc/launcher.md Outdated Show resolved Hide resolved
@nharrand
Copy link
Collaborator Author

I deactivated travis builds for spoon-bytecode because it relies on resolving spoon-core which in turn has spoon-pom as a parent, which is not publish yet, so it fails. We ca just uncomment the two builds in a futur PR. Otherwise, this is ready for review.

@nharrand nharrand changed the title WiP: refactor: Excluding decompiler related features from spoon-core refactor: Excluding decompiler related features from spoon-core Nov 17, 2018
@nharrand
Copy link
Collaborator Author

Similarly properties could be move into spoon-pom, but ... #2771

@surli
Copy link
Collaborator

surli commented Nov 18, 2018

FTR I got an email from the CFR maintainer:

CFR 0.135 (damnit, I even changed my naming scheme to conform, ish. ;) ) is now available on maven central.

<dependency>
  <groupId>org.benf</groupId>
  <artifactId>cfr</artifactId>
  <version>0.135</version>
</dependency>

@nharrand
Copy link
Collaborator Author

Except for the remaining discussion on the name (and possibly, the README.md). This PR is ready for review.

spoon-pom/pom.xml Outdated Show resolved Hide resolved
spoon-bytecode/pom.xml Outdated Show resolved Hide resolved
@pvojtechovsky
Copy link
Collaborator

I do not use Maven and I don't try to understand that building stuff, but I like the idea of this PR a lot. 👍 Thank You Nicolas!

.travis.yml Outdated Show resolved Hide resolved
spoon-bytecode/pom.xml Outdated Show resolved Hide resolved
spoon-bytecode/pom.xml Outdated Show resolved Hide resolved
spoon-bytecode/pom.xml Outdated Show resolved Hide resolved
@surli
Copy link
Collaborator

surli commented Nov 21, 2018

Sorry for asking lot of changes but the idea here is really to have a first external module of Spoon to start seeing how we can decompose some parts, so the most info we can put in an external pom and reuse, the better.
By the way, a good thing you can try before we merge the PR is a dry-run mvn release, on spoon-pom: mvn release:prepare -DdryRun=true, this will check that everything's ok to release all spoon artefacts at once. Thanks

@nharrand
Copy link
Collaborator Author

nharrand commented Nov 21, 2018

Sorry for asking lot of changes but the idea here is really to have a first external module of Spoon to start seeing how we can decompose some parts, so the most info we can put in an external pom and reuse, the better.

Don't be, it's actually helpful. But, to answer a lot of your comments, my problem is that spoon-pom is not yet published on maven central. So, locally on my machine I can move a lot of things in spoon-pom, but if I do, I'll break CI. So I was trying to achieve an ugly middle ground where I add stuff to spoon-pom, planing to remove them from other poms later.
Do you have an other solution? (Should I not care to break CI, if everything run smoothly locally?)

@surli
Copy link
Collaborator

surli commented Nov 21, 2018

Do you have an other solution? (Should I not care to break CI, if everything run smoothly locally?

Change CI scripts to run directly all tests from spoon-pom.

@spoon-bot
Copy link
Collaborator

API changes: 7 (Detected by Revapi)

Old API: fr.inria.gforge.spoon:spoon-core:jar:7.2.0-20181120.234134-99 / New API: fr.inria.gforge.spoon:spoon-core:jar:7.2.0-SNAPSHOT

Class was removed.
Old class JarLauncher
New none
Breaking binary: breaking
Class was removed.
Old class CFRDecompiler
New none
Breaking binary: breaking
Class was removed.
Old interface Decompiler
New none
Breaking binary: breaking
Class was removed.
Old class FernflowerDecompiler
New none
Breaking binary: breaking
Class was removed.
Old class MultiTypeTransformer
New none
Breaking binary: breaking
Class was removed.
Old class SpoonClassFileTransformer
New none
Breaking binary: breaking
Class was removed.
Old interface TypeTransformer
New none
Breaking binary: breaking

pom.xml Outdated Show resolved Hide resolved
pom.xml Show resolved Hide resolved
</executions>
</plugin>

<plugin>
Copy link
Collaborator

Choose a reason for hiding this comment

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

you removed checkstyle from spoon-core, so I assume you put it in spoon-pom, so it means you don't need it here.

Copy link
Collaborator Author

@nharrand nharrand Nov 21, 2018

Choose a reason for hiding this comment

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

No. Both spoon-core and spoon-decompiler declare the checkstyle plugin but with differents options (exculsion of some sources for exemple).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But the checkstyle profile is now in spoon-pom.

@monperrus
Copy link
Collaborator

We did the last changes per our discussion, CI is green.

@surli do you merge?

@surli surli merged commit 88f909d into INRIA:master Nov 26, 2018
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.

5 participants