-
-
Notifications
You must be signed in to change notification settings - Fork 39
fix: add newChangesetTemplateFallback
for newer GitLab versions
#216
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
Conversation
🦋 Changeset detectedLatest commit: 131826c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update introduces a fallback changeset template to address an issue where GitLab ignores query strings in file creation links, resulting in blank editors. The fallback template is appended to comment messages, allowing users to manually copy and paste the suggested changeset filename and content. The implementation modifies the comment generation functions to include this fallback and updates how the changeset template is constructed and encoded. Additionally, the project replaces Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommentModule
participant GitLab
User->GitLab: Opens changeset creation link
GitLab-->>User: Editor opens (may be blank if query ignored)
User->CommentModule: Views PR comment
CommentModule->User: Shows link and fallback template (filename + content)
User->GitLab: Manually copies fallback template if needed
Assessment against linked issues
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/comment.tsOops! Something went wrong! :( ESLint: 9.24.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/comment.ts:298
- Verify that the generated newChangesetFileName string correctly integrates the humanId function output with the intended path separator. Any misformatting could lead to an incorrect file path when constructing the changeset URL.
const newChangesetFileName = `.changeset/${humanId({
src/comment.ts:314
- Ensure that the fallback template string formatting is exactly as intended when injected into the comment. Minor newline or spacing issues may affect the readability of the rendered changeset instructions.
const newChangesetTemplateFallback = `
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
📊 Package size report 1%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 84d2931 in 2 minutes and 43 seconds
More details
- Looked at
188
lines of code in5
files - Skipped
1
files when reviewing. - Skipped posting
17
drafted comments based on config settings.
1. .nano-staged.js:1
- Draft comment:
New .nano-staged.js file added referencing '@1stg/nano-staged/tsc'. Ensure this is the intended replacement for lint-staged and that any necessary configurations are updated accordingly. - Reason this comment was not posted:
Confidence changes required:20%
<= threshold50%
None
2. package.json:77
- Draft comment:
Dependency versions updated and lint-staged replaced by nano-staged. Verify that switching to nano-staged does not break any local workflows and that all peer dependencies are compatible. - Reason this comment was not posted:
Confidence changes required:30%
<= threshold50%
None
3. src/comment.ts:74
- Draft comment:
Added the new parameter 'newChangesetTemplateFallback' in message templates. Ensure that its inclusion in the message provides clear fallback instructions for GitLab >16.11. - Reason this comment was not posted:
Confidence changes required:30%
<= threshold50%
None
4. src/comment.ts:118
- Draft comment:
Likewise, the updated 'getApproveMessage' function now includes 'newChangesetTemplateFallback'. Confirm that all call sites provide this new argument consistently. - Reason this comment was not posted:
Confidence changes required:30%
<= threshold50%
None
5. src/comment.ts:303
- Draft comment:
Removed encodeURIComponent from getNewChangesetTemplate function and now applying it when constructing addChangesetUrl. This improves clarity, but double-check that encoding is applied consistently everywhere needed. - Reason this comment was not posted:
Confidence changes required:20%
<= threshold50%
None
6. src/comment.ts:314
- Draft comment:
The fallback message clearly instructs users on manual copy/paste if GitLab regression occurs. Verify that the formatting and link to the GitLab issue remains current. - Reason this comment was not posted:
Confidence changes required:20%
<= threshold50%
None
7. src/context.ts:1
- Draft comment:
Minor stylistic change from block comment to single-line comment; ensure that this doesn't impact the simulated context in GitHub Actions. - Reason this comment was not posted:
Confidence changes required:10%
<= threshold50%
None
8. .nano-staged.js:1
- Draft comment:
New file looks good. Consider ensuring there’s a trailing newline at the end of the file for best practices. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
9. package.json:101
- Draft comment:
Dependency update: 'lint-staged' has been removed and replaced with 'nano-staged'. Ensure that any tool or CI configuration referencing lint-staged (or .lintstagedrc.js) is updated accordingly. - Reason this comment was not posted:
Comment looked like it was already resolved.
10. package.json:113
- Draft comment:
The resolution for 'es5-ext' has been removed. Confirm this removal is intentional and that it does not lead to dependency conflicts in downstream packages. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
11. src/comment.ts:74
- Draft comment:
Added new parameter 'newChangesetTemplateFallback' to getAbsentMessage to provide fallback instructions for GitLab >16.11 regression. Ensure related documentation and tests are updated. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that documentation and tests are updated, which violates the rule against asking the author to ensure things are tested or documented. The comment does not provide a specific suggestion or point out a specific issue with the code.
12. src/comment.ts:91
- Draft comment:
Similarly, 'newChangesetTemplateFallback' was added to getApproveMessage. Make sure that downstream usage and any documentation reflect this new parameter. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. src/comment.ts:126
- Draft comment:
Refactored getNewChangesetTemplate to remove in-function URL encoding. This is a good separation of concerns—but ensure all callers encode the returned template when embedding it in URLs. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that all callers encode the returned template after a refactoring change. This is a general request to ensure something is done, which violates the rule against asking the author to ensure behavior is intended or tested.
14. src/comment.ts:298
- Draft comment:
The creation of a unique changeset file name using 'humanId' and the construction of the new template are correctly implemented. Double-check that any special characters in the title are handled by the later encodeURIComponent call. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to double-check something, which violates the rule against asking the author to confirm or ensure behavior. It doesn't provide a specific suggestion or point out a clear issue.
15. src/comment.ts:314
- Draft comment:
The fallback template message provides clear guidance for the GitLab >16.11 regression. Verify that its markdown formatting (especially the fenced code block) renders as expected in GitLab. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is asking the PR author to verify the markdown formatting, which falls under the category of asking for confirmation or verification. This violates the rule against asking the PR author to confirm or ensure behavior.
16. src/context.ts:5
- Draft comment:
Exporting 'ref' from context.ts can be useful. Consider adding runtime checks or more descriptive error messages if CI_COMMIT_REF_NAME isn’t set, similar to projectId. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
17. src/comment.ts:314
- Draft comment:
Consider rephrasing the fallback message text from "fill the changeset template file name and content" to something like "pre-fill" or "populate the changeset template's filename and content" for improved clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
This is a pure wording suggestion that doesn't affect functionality. While "pre-fill" might be marginally clearer than "fill", the original wording is already understandable. The comment is about UI text, not logic. The suggestion is not fixing a real issue or improving code quality in a meaningful way.
The suggested wording is arguably more precise and professional. Poor UI text can impact user experience.
While better wording is good, this change is too minor to warrant a comment. The original text is clear enough for users to understand the functionality.
Delete the comment. The wording suggestion is too minor and doesn't address code logic or quality.
Workflow ID: wflow_L0EpghFqLCxsExBu
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (6)
.changeset/healthy-insects-promise.md
(1 hunks).lintstagedrc.js
(0 hunks).nano-staged.js
(1 hunks)package.json
(2 hunks)src/comment.ts
(5 hunks)src/context.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- .lintstagedrc.js
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/comment.ts (1)
src/env.ts (1)
env
(10-33)
🪛 GitHub Check: Lint and Test with Node.js 18
.changeset/healthy-insects-promise.md
[warning] 7-7:
{"reason":"Unexpected GFM autolink literal, expected regular autolink, add <
before and >
after","source":"remark-lint","ruleId":"no-literal-urls","severity":1}
🪛 GitHub Check: Lint and Test with Node.js 22
.changeset/healthy-insects-promise.md
[warning] 7-7:
{"reason":"Unexpected GFM autolink literal, expected regular autolink, add <
before and >
after","source":"remark-lint","ruleId":"no-literal-urls","severity":1}
🪛 GitHub Check: Lint and Test with Node.js 20
.changeset/healthy-insects-promise.md
[warning] 7-7:
{"reason":"Unexpected GFM autolink literal, expected regular autolink, add <
before and >
after","source":"remark-lint","ruleId":"no-literal-urls","severity":1}
🪛 markdownlint-cli2 (0.17.2)
.changeset/healthy-insects-promise.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (14)
src/context.ts (1)
1-1
: LGTM: Minor change to comment styleThe change from JSDoc-style comment to a single-line inline comment doesn't affect functionality.
.nano-staged.js (1)
1-1
: LGTM: Proper replacement for lint-staged configurationThis new file correctly re-exports from
@1stg/nano-staged/tsc
, replacing the previous.lintstagedrc.js
that re-exported from@1stg/lint-staged
. This aligns with the switch fromlint-staged
tonano-staged
in package.json.package.json (4)
76-76
: LGTM: Dependency update for dotenvUpdating dotenv from 16.4.7 to 16.5.0 is a minor version bump that likely includes bug fixes and improvements.
91-94
: LGTM: Development dependencies updatedThe updates to
@1stg/common-config
(12.0.0 → 13.0.1) and@changesets/cli
(2.28.1 → 2.29.0) are appropriate and keep the development tooling current.
95-98
: LGTM: Minor development dependency updatesUpdates to
@pkgr/rollup
and@types/web
are minor version bumps that likely include bug fixes and type improvements.
102-102
: LGTM: Replacement of lint-staged with nano-stagedThis change aligns with the new
.nano-staged.js
file and removes the dependency onlint-staged
, replacing it withnano-staged
v0.8.0.src/comment.ts (8)
74-78
: LGTM: Added fallback template parameterThe
newChangesetTemplateFallback
parameter has been added to thegetAbsentMessage
function to provide a manual template option when the URL parameter approach fails in newer GitLab versions.
95-96
: LGTM: Template fallback incorporated into absent messageThe fallback template is now properly integrated into the absent message body, giving users a manual option when GitLab doesn't correctly prefill the template via URL parameters.
100-104
: LGTM: Added fallback template parameter to getApproveMessageThe
newChangesetTemplateFallback
parameter has been consistently added to both message generation functions.
121-122
: LGTM: Template fallback incorporated into approve messageThe fallback template is consistently included in both message types.
126-134
: LGTM: Improved template generationThe
getNewChangesetTemplate
function has been updated to return a raw template string instead of an encoded URI component, with encoding moved to the usage site. This makes the function more reusable.
298-312
: LGTM: Extracted filename and template generation for reuseThe changeset filename and template construction have been extracted into constants for better reuse, which improves code maintainability. The encoding of the template is now correctly handled at the URL construction site.
314-320
: LGTM: Well-designed fallback template with clear instructionsThe fallback template provides clear instructions with a reference to the GitLab issue, and includes proper markdown formatting with a code block for easy copying. This is a good solution for the GitLab regression.
323-335
: LGTM: Updated message generation to include fallback templateThe message generation now properly includes the fallback template for both approve and absent cases.
close #178
related https://gitlab.com/gitlab-org/gitlab/-/issues/532221
Important
Adds
newChangesetTemplateFallback
for GitLab >= 16.11 regression and updates dependencies.newChangesetTemplateFallback
incomment.ts
to handle GitLab >= 16.11 regression by providing a manual template copy option.getAbsentMessage()
andgetApproveMessage()
to includenewChangesetTemplateFallback
.dotenv
to^16.5.0
and@1stg/common-config
to^13.0.1
inpackage.json
.lint-staged
withnano-staged
inpackage.json
..lintstagedrc.js
with.nano-staged.js
.This description was created by
for 84d2931. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Chores
Style