docs: correct the mvn-toolchain-id default in README and action.yml - #1207
Merged
Conversation
The generated toolchain ID is `${vendor}_${version}`, where vendor is the
mvn-toolchain-vendor input falling back to distribution. Two places described
this incorrectly.
action.yml claimed the default was "${distribution}_${java-version}", which
is wrong whenever mvn-toolchain-vendor is set, since overriding the vendor
also changes the generated ID.
The README used `${vendor}`, which is accurate but names something that is
not an action input, leaving readers to guess where the value comes from.
Both now name mvn-toolchain-vendor and state that it falls back to
distribution.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 664777db-7250-417d-b94d-d5529ec3fec2
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes and aligns the documentation for how setup-java derives the default Maven toolchain id, matching the actual implementation in src/toolchains.ts (default id is ${vendor}_${version}, where vendor comes from the mvn-toolchain-vendor input and falls back to distribution).
Changes:
- Update
README.mdto name themvn-toolchain-vendorinput explicitly in the default toolchain ID description. - Update
action.yml’smvn-toolchain-idinput description to reflect the correct default ID format and note the vendor fallback todistribution.
Show a summary per file
| File | Description |
|---|---|
| README.md | Clarifies the default Maven toolchain ID template by referencing mvn-toolchain-vendor explicitly. |
| action.yml | Corrects the mvn-toolchain-id input description to match the real default ID derivation and vendor fallback behavior. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
The generated Maven toolchain ID is
${vendor}_${version}, wherevendoris themvn-toolchain-vendorinput falling back todistribution(src/toolchains.ts). Two places described this incorrectly, and they disagreed with each other:action.ymlclaimed the default was"${distribution}_${java-version}". That is wrong whenevermvn-toolchain-vendoris set, because overriding the vendor also changes the generated ID. A user who sets a vendor and then writes a toolchain requirement against the distribution-based ID gets a toolchain that never matches.README.mdsaid${vendor}_${java-version}. That is accurate, butvendoris not the name of any action input, so it left readers to guess where the value came from.Both now name
mvn-toolchain-vendorand state that it falls back todistribution.docs/advanced-usage.mdalready explained this correctly and is unchanged.Found via a Copilot review comment on #1205, which flagged the README wording; checking it against the source showed
action.ymlwas the more misleading of the two.Related issue:
N/A. Follow-up to #1204.
Check list:
npm run checklocally (format, lint, build, test) and all checks pass. Not run. The change is two description strings;action.ymlis metadata that is not compiled intodist/, and I verified it still parses as YAML.