fix(core/github): edDraftURI opt-out and custom newIssuesURL#5246
fix(core/github): edDraftURI opt-out and custom newIssuesURL#5246marcoscaceres wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the GitHub integration in ReSpec’s core config normalization so specs can customize the “new issue” destination and aims to support opting out of the GitHub-derived edDraftURI. It also extends the GitHub module’s localized strings and adds/updates tests around the new config behavior.
Changes:
- Add support for
github.newIssuesURLand use it when generating GitHub feedback links. - Adjust
core/githubhandling aroundedDraftURIgeneration when the config explicitly provides that field. - Add GitHub spec tests for
edDraftURIopt-out cases and customnewIssuesURL, plus French localization strings incore/github.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/core/github.js |
Adds French strings, introduces custom newIssuesURL parsing/validation, and changes edDraftURI defaulting logic. |
tests/spec/core/github-spec.js |
Adds tests for explicit edDraftURI values and normalization of custom newIssuesURL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| typeof conf.github === "object" && | ||
| conf.github.hasOwnProperty("newIssuesURL") | ||
| ) { | ||
| try { | ||
| const url = new URL(String(conf.github.newIssuesURL)); | ||
| if (url.protocol !== "https:") { | ||
| const msg = docLink`${"[github.newIssuesURL]"} must use HTTPS. (${String(conf.github.newIssuesURL)}).`; | ||
| rejectGithubPromise(msg); | ||
| return; | ||
| } | ||
| newIssuesURL = url.href; |
There was a problem hiding this comment.
The HTTPS-only check is intentionally broad. Monorepo scenarios use custom issue URLs at arbitrary paths (e.g., label-filtered queries). Tightening to require /issues/ would break legitimate use cases.
| it("supports custom newIssuesURL", async () => { | ||
| const opts = { | ||
| config: Object.assign(makeBasicConfig(), { | ||
| github: { | ||
| repoURL: "https://github.com/speced/respec/", | ||
| newIssuesURL: "https://github.com/speced/respec/issues/new", | ||
| }, | ||
| }), | ||
| body: makeDefaultBody(), | ||
| }; | ||
| delete opts.config.edDraftURI; | ||
| const doc = await makeRSDoc(opts); | ||
| const { respecConfig: conf } = doc.defaultView; | ||
| expect(conf.github.newIssuesURL).toBe( | ||
| "https://github.com/speced/respec/issues/new" | ||
| ); |
There was a problem hiding this comment.
Added. The test now asserts the rendered feedback link in the DOM uses the custom URL, not just the config value.
There was a problem hiding this comment.
Done in ce9b996. The test now adds excludeGithubLinks: false so the links are rendered, then asserts that the "File an issue" <dd> element's <a> href equals the custom newIssuesURL — same pattern as the existing pullsURL/commitHistoryURL monorepo test.
| if (!conf.hasOwnProperty("edDraftURI")) { | ||
| newProps.edDraftURI = `https://${org.toLowerCase()}.github.io/${repo}/`; |
There was a problem hiding this comment.
The opt-out IS implemented. The hasOwnProperty("edDraftURI") check at the bottom of the diff:
if (!conf.hasOwnProperty("edDraftURI")) {
newProps.edDraftURI = `https://${org}.github.io/${repo}/`;
}Only auto-generates edDraftURI when the author hasn't set it. If they set edDraftURI: null or edDraftURI: "", hasOwnProperty is true and the auto-generation is skipped. Two new tests verify both paths.
|
@copilot Can you confirm all your feedback has been addressed? Can you fix anything that was not addressed? |
… link Agent-Logs-Url: https://github.com/speced/respec/sessions/20f04c30-6520-406b-b673-555dabf64b8b Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
All three reviewer concerns have been addressed:
|
Closes #5103
Closes #4317