Skip to content

Add GenerateUnpickData task#27

Merged
sciwhiz12 merged 2 commits intoParchmentMC:devfrom
Earthcomputer:unpick
Jan 20, 2025
Merged

Add GenerateUnpickData task#27
sciwhiz12 merged 2 commits intoParchmentMC:devfrom
Earthcomputer:unpick

Conversation

@Earthcomputer
Copy link
Copy Markdown
Contributor

This is the only modification to Compass required to start shipping unpick data with Parchment

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jan 16, 2025

CLA assistant check
All committers have signed the CLA.

@marchermans
Copy link
Copy Markdown
Member

What is the end goal here?
This task combines the unpick files from a directory into one?
Is there a reason this is needed?

@Earthcomputer
Copy link
Copy Markdown
Contributor Author

Earthcomputer commented Jan 16, 2025

The goal is to publish unpick files with Parchment. It is more convenient to develop unpick files separately but it's nice to bundle them into one when publishing. This is what Yarn does, and Parchment already does this with mappings. There is no technical reason it has to be done this way.

@sciwhiz12 sciwhiz12 added the enhancement New feature or request label Jan 18, 2025
Copy link
Copy Markdown
Member

@sciwhiz12 sciwhiz12 left a comment

Choose a reason for hiding this comment

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

Looks generally fine, just one minor quibble about the use of try-catch versus method throws declaration.

Comment thread src/main/java/org/parchmentmc/compass/tasks/GenerateUnpickData.java Outdated
Copy link
Copy Markdown
Member

@sciwhiz12 sciwhiz12 left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@sciwhiz12 sciwhiz12 merged commit 8afae3e into ParchmentMC:dev Jan 20, 2025
@Earthcomputer Earthcomputer deleted the unpick branch January 20, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants