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

Drop Quiltflower and upgrade to Vineflower that is the replacement #100

Merged
merged 2 commits into from Nov 11, 2023

Conversation

RoiSoleil
Copy link

Fix #99

@RoiSoleil
Copy link
Author

@jpstotz Could you review my PR ?

@jpstotz jpstotz self-assigned this Nov 8, 2023
@jpstotz
Copy link
Collaborator

jpstotz commented Nov 8, 2023

Thanks for the PR. I noticed that you included the whole Vineflower JAR file, exactly as it was done with Quiltflower.

On the long run I would try to get away from this way of adding external libraries in ECD (where possible). Since I added Quiltflower I found a way that seem to work better as it just uses Maven to download the official artifact and place it in the lib folder.

As an example please see how I implemented it for Fernflower https://github.com/ecd-plugin/ecd/blob/master/org.sf.feeling.decompiler/pom.xml

As Vineflower provides their artifacts via Maven central I would also prefer to reference only this artifact instead of including it into the ecd git repo.

Could you please see if you can adapt this way in your PR for Vineflower?

@RoiSoleil
Copy link
Author

This is done ;)

@jpstotz jpstotz merged commit 5e00dd2 into ecd-plugin:master Nov 11, 2023
@jpstotz
Copy link
Collaborator

jpstotz commented Nov 11, 2023

@RoiSoleil Thanks for providing the PR.

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.

Drop Quiltflower and upgrade to Vineflower that is the replacement
2 participants