fix(w3c/sotd): Adjust pluralization of "Working Group" for CRD/CRYD#5166
fix(w3c/sotd): Adjust pluralization of "Working Group" for CRD/CRYD#5166kfranqueiro wants to merge 3 commits intospeced:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the W3C SoTD (Status of This Document) template logic so CRD/CRYD “Working Group” wording can be pluralized when a spec is published by multiple W3C groups, aligning output with pubrules expectations.
Changes:
- Adjust CRD and CRYD
statusExplanationtext to switch between “Working Group intends” vs “Working Groups intend”. - Expand W3C group-related tests to assert singular/plural wording for CRD/CRYD.
- Fix a minor grammar issue in an existing test name.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/w3c/templates/sotd.js |
Adds conditional pluralization for “Working Group(s) … intend(s)” in CRD/CRYD status explanation text. |
tests/spec/w3c/group-spec.js |
Adds assertions for CRD/CRYD singular vs plural WG phrasing; fixes a test description typo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| statusExplanation = html`A Candidate Recommendation Draft integrates | ||
| changes from the previous Candidate Recommendation that the Working Group | ||
| intends to include in a subsequent Candidate Recommendation Snapshot.`; | ||
| changes from the previous Candidate Recommendation that the Working | ||
| Group${Array.isArray(conf.wg) ? "s intend" : " intends"} to include in a | ||
| subsequent Candidate Recommendation Snapshot.`; |
There was a problem hiding this comment.
Pluralization here keys off Array.isArray(conf.wg), which will incorrectly pluralize when the user provides a single group as an array (e.g., group: ["payments"] => conf.wg becomes an array of length 1). Since this template already relies on conf.multipleWGs elsewhere, use conf.multipleWGs (or Array.isArray(conf.wg) && conf.wg.length > 1) to decide between “Group intends” vs “Groups intend”.
| statusExplanation = html`A Candidate Registry Draft integrates changes | ||
| from the previous Candidate Registry Snapshot that the Working Group | ||
| intends to include in a subsequent Candidate Registry Snapshot.`; | ||
| from the previous Candidate Registry Snapshot that the Working | ||
| Group${Array.isArray(conf.wg) ? "s intend" : " intends"} to include in a | ||
| subsequent Candidate Registry Snapshot.`; |
There was a problem hiding this comment.
Same issue as the CRD branch: Array.isArray(conf.wg) will pluralize even for a single WG if it was configured via an array. Switch this conditional to conf.multipleWGs (or check conf.wg.length > 1) so pubrules-style pluralization is based on the actual number of working groups, not the value type.
| const sotd = doc.getElementById("sotd").textContent.replace(/\s+/g, " "); | ||
| expect(sotd).toContain("that the Working Group intends to include in"); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The new CRD/CRYD SotD tests cover string vs multi-element array, but they don’t cover the common edge case where a single group is provided as an array (e.g., group: ["payments"]). Adding that case would prevent regressions where pluralization is based on Array.isArray(conf.wg) instead of the actual group count.
| it(`when one group is specified as an array in a ${specStatus}, "Working Group" is singular`, async () => { | |
| const ops = makeStandardOps({ | |
| group: ["payments"], | |
| specStatus, | |
| }); | |
| const doc = await makeRSDoc(ops); | |
| const sotd = doc.getElementById("sotd").textContent.replace(/\s+/g, " "); | |
| expect(sotd).toContain("that the Working Group intends to include in"); | |
| }); |
There was a problem hiding this comment.
At first I had intentionally not done this as it seemed there were other places in the codebase that already exhibit the same behavior, but upon pulling further on this thread I see that headers.js does in fact consider this, and is responsible for assigning config.multipleWGs, which sotd.js uses in one other location, so I can use that for this PR as well.
marcoscaceres
left a comment
There was a problem hiding this comment.
Suggestion for cleaner it() descriptions — shorter, specStatus leads, and the quoted text matches what's actually asserted.
marcoscaceres
left a comment
There was a problem hiding this comment.
Co-pilot caught a couple of good ones... might be worth adding tests for those too.
61beaf8 to
df895e5
Compare
|
@kfranqueiro can you give it a rebase? 🙏 |
df895e5 to
8aba777
Compare
Fixes #5164.
This adds a condition to the value of
statusExplanationdependent on whether the document lists a single WG or multiple WGs, so that "Working Group" is properly pluralized in the latter case.This logic is in line with what pubrules checks for.