feat: add optional GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE env
#196
Conversation
🦋 Changeset detectedLatest commit: e5b8119 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 |
|
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. |
JounQin
changed the title
GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE env
JounQin
force-pushed
the
add-discussion-resolve
branch
from
e57dece to
f8630a7
Compare
WalkthroughThe pull request introduces a new environment variable, Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI/CD Pipeline
participant CH as Comment Handler
participant Env as Environment Config
participant API as GitLab API
CI->>CH: Process changeset for merge request
CH->>Env: Retrieve env variables (incl. GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE)
alt Changeset detected
CH->>Env: Evaluate auto-resolve flag via TRUTHY_VALUES
alt Flag is truthy
CH->>API: Trigger discussion resolution
else Flag is false
CH->>API: Post note comment
end
else No changeset
CH->>API: Create discussion comment for missing changeset
end
Assessment against linked issues
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/utils.tsOops! Something went wrong! :( ESLint: 9.23.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js src/comment.tsOops! Something went wrong! :( ESLint: 9.23.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js src/main.tsOops! Something went wrong! :( ESLint: 9.23.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js
✨ Finishing Touches
🪧 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 (
|
commit: |
|
Thanks @crysadrak for your contribution! I changed the env name to If you want to always resolve the discussion, you should actually use |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/main.ts (1)
24-68: 💡 Verification agent🧩 Analysis chain
Missing implementation for GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE
The PR introduces the type definition for
GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE, but its actual implementation seems to be missing from the files under review. Based on the AI summary, this might be implemented insrc/comment.ts, which is not included in this review.
🌐 Web query:
Can you verify if src/comment.ts in the changesets-gitlab repository contains the implementation for GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE?💡 Result:
The
src/comment.tsimplementation in changesets-gitlab does not includeGITLAB_COMMENT_DISCUSSION_AUTO_RESOLVEbased on available documentation[3][7]. The closest related feature is theGITLAB_COMMENT_TYPEenvironment variable introduced in v0.9.0, which lets users choose between GitLab's discussion threads (discussion) or individual notes (note) for comments[7].Key points from the search results:
- Input variables include
INPUT_COMMENTfor commit messages andGITLAB_COMMENT_TYPEfor comment format selection[3]- The changelog[7] shows no implementation of auto-resolve functionality for discussions
- Issue tracking[6] mentions basic commenting capabilities but no resolution automation
For comment management, the tool focuses on MR interaction rather than discussion thread resolution workflows[1][3][7]. Developers would need to handle discussion resolution through GitLab's native mechanisms or custom scripting.
Citations:
- 1: https://www.npmjs.com/package/changesets-gitlab/v/0.5.1
- 2: https://www.npmjs.com/package/changesets-gitlab/v/0.2.0
- 3: https://github.com/un-ts/changesets-gitlab
- 4: https://gitlab.com/gitlab-org/gitlab/-/issues/31978
- 5: https://dev.to/wdsebastian/simplest-way-to-publish-and-automate-npm-packages-d0c
- 6: https://www.drupal.org/project/api_client/issues/3423684
- 7: https://github.com/un-ts/changesets-gitlab/blob/main/CHANGELOG.md
- 8: https://raw.githubusercontent.com/rx-ts/changesets-gitlab/056d8c1c73ba26ead5046378b6bf5a3bc31aaa3a/CHANGELOG.md
- 9: https://app.renovatebot.com/package-diff?name=changesets-gitlab&from=0.12.0&to=0.12.1
- 10: https://opensource.unicc.org/openwallet-foundation/credo-ts/-/tree/v0.5.11/.changeset
Review Notice: Missing Implementation for Auto-Resolve Feature
The PR introduces the type definition for
GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE, but verification confirms that there is no corresponding implementation insrc/comment.ts. Instead, the current functionality only supports comment type selection through theGITLAB_COMMENT_TYPEenvironment variable. Please clarify whether the auto-resolve feature should be implemented or if the type definition is no longer needed, and adjust accordingly.
- Action Required:
- Implement the auto-resolve functionality in
src/comment.tsif intended, or remove the unused type definition to prevent confusion.
🧹 Nitpick comments (2)
.changeset/swift-berries-exist.md (1)
5-5: Improve changeset description readabilityThe description is informative but could benefit from improved formatting with proper punctuation to enhance readability.
-feat: add a new optional `GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE` environment variable to automatically resolve added discussion when changeset is present, if you want always resolve the discussion, you should actually use `GITLAB_COMMENT_TYPE=note` instead, default `true` +feat: add a new optional `GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE` environment variable to automatically resolve added discussion when changeset is present. If you want to always resolve the discussion, you should use `GITLAB_COMMENT_TYPE=note` instead. Default value is `true`.src/comment.ts (1)
312-323: New feature: Auto-resolving discussions when changeset exists.This implementation checks if a changeset exists and if auto-resolving is enabled (defaulting to true with '1'). The conditional can be simplified since
TRUTHY_VALUESalready contains '1', making the default truthy.if ( hasChangeset && - TRUTHY_VALUES.has(env.GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE || '1') + TRUTHY_VALUES.has(env.GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE ?? '1') ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/swift-berries-exist.md(1 hunks)README.md(1 hunks)src/comment.ts(6 hunks)src/env.ts(0 hunks)src/main.ts(3 hunks)src/types.ts(1 hunks)src/utils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/env.ts
🧰 Additional context used
🧬 Code Definitions (2)
src/main.ts (4)
src/types.ts (1)
MainCommandOptions(1-4)src/env.ts (1)
env(10-33)src/git-utils.ts (1)
setupUser(6-13)src/utils.ts (1)
TRUTHY_VALUES(175-175)
src/comment.ts (2)
src/env.ts (1)
env(10-33)src/utils.ts (1)
TRUTHY_VALUES(175-175)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (14)
src/utils.ts (1)
175-175: Clean addition of TRUTHY_VALUES utilityThe new constant
TRUTHY_VALUESis a sensible addition that will provide a consistent way to check truthy string values across the codebase. Using aSetis an efficient choice for frequent lookups..changeset/swift-berries-exist.md (1)
1-5: LGTM - Minor version bump is appropriateThis changeset correctly identifies the change as a minor version bump, which is appropriate for adding a new feature without breaking existing functionality.
src/types.ts (1)
25-25: Making DEBUG_GITLAB_CREDENTIAL optional is a good improvementChanging
DEBUG_GITLAB_CREDENTIALto be optional aligns with its usage pattern in the codebase and provides better type safety.src/main.ts (3)
13-18: Import of new TRUTHY_VALUES utilityThe addition of the
TRUTHY_VALUESimport improves code organization by centralizing the definition of what constitutes a truthy value in environment variables.
24-24: Simplified environment variable destructuringReducing the number of destructured variables improves maintainability by making it clearer when environment variables are being accessed.
29-29: Direct access to environment variablesConsistently accessing properties through the
envobject makes the code more predictable and easier to trace.Also applies to: 33-33, 48-48, 68-68
src/comment.ts (5)
23-23: Added import for the new auto-resolve feature.The
TRUTHY_VALUESimport is used to determine whether discussions should be automatically resolved based on the new environment variable setting.
249-251: Good variable renaming for clarity.Renaming from environment variable names to more concise variable names improves readability and follows better variable naming conventions.
261-261: Direct reference to environment variable instead of destructured variable.Using the environment variable directly is consistent with the approach taken for other references in this code segment.
289-301: Improved consistency in environment variable access.Direct references to
envproperties instead of using destructured variables improves consistency throughout the codebase and ensures the most up-to-date values are used.
355-355: Consistent variable name in error message.The error message correctly references the renamed
commentTypevariable.README.md (3)
51-55: Documentation formatting improvements.Consistent spacing and improved documentation formatting make the README more readable and professional.
56-56: Documentation for the new auto-resolve feature.The documentation clearly explains the purpose of the new
GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVEenvironment variable and its default behavior. It also points out that usingGITLAB_COMMENT_TYPE=noteis an alternative approach if users want to always resolve discussions.
57-58: Consistent formatting for remaining environment variables.Continuing the formatting improvements for the remaining environment variables enhances the overall documentation quality.
@coderabbitai Please run |
JounQin
force-pushed
the
add-discussion-resolve
branch
from
f8630a7 to
e5b8119
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/swift-berries-exist.md (1)
1-5: Changeset accurately documents the new feature.The changeset correctly describes the new environment variable
GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVEand its purpose. It provides clear information about the default value and alternative options.Consider using more formal language in the documentation.
For better readability and professionalism, consider rephrasing "if you want to" to something like "if you prefer to" or "to always resolve discussions".
-feat: add a new optional `GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE` environment variable to automatically resolve added discussion when changeset is present, if you want to always resolve the discussion, you should actually use `GITLAB_COMMENT_TYPE=note` instead, default `true` +feat: add a new optional `GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE` environment variable to automatically resolve added discussion when changeset is present. To always resolve discussions, use `GITLAB_COMMENT_TYPE=note` instead. Default value is `true`🧰 Tools
🪛 LanguageTool
[style] ~5-~5: Consider using a more polite wording.
Context: ...ssion when changeset is present, if you want to always resolve the discussion, you s...(IF_YOU_WANT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/swift-berries-exist.md(1 hunks)README.md(1 hunks)src/comment.ts(6 hunks)src/env.ts(0 hunks)src/main.ts(3 hunks)src/types.ts(1 hunks)src/utils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/env.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/utils.ts
- README.md
- src/main.ts
- src/types.ts
🧰 Additional context used
🧬 Code Definitions (1)
src/comment.ts (2)
src/env.ts (1)
env(10-33)src/utils.ts (1)
TRUTHY_VALUES(175-175)
🪛 LanguageTool
.changeset/swift-berries-exist.md
[style] ~5-~5: Consider using a more polite wording.
Context: ...ssion when changeset is present, if you want to always resolve the discussion, you s...
(IF_YOU_WANT)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (6)
src/comment.ts (6)
23-23: Appropriate import of TRUTHY_VALUES utility.The import adds the necessary utility to check for truthy values in the environment variable.
247-251: Variable renaming improves code clarity.Renaming environment variables to more concise local variables (
GITLAB_COMMENT_TYPE→commentTypeandGITLAB_ADD_CHANGESET_MESSAGE→commitMessage) enhances readability. The removal of other destructured variables in favor of direct access viaenvobject later in the code is also a good practice as it centralizes environment variable access.
261-261: Direct access to environment variable is consistent with the refactoring.This change aligns with the earlier refactoring that removed destructured variables in favor of direct access through the
envobject.
289-301: Consistent use of env object for environment variables.The changes in this string construction are aligned with the earlier refactoring, using direct access to the
envobject properties instead of destructured variables. The conditional forcommitMessageis also appropriately updated.
355-355: Error message updated to match variable name.The error message has been correctly updated to use
commentTypeinstead of the previous environment variable name.
309-324:✅ Verification successful
Well-implemented auto-resolve feature for discussions.
The new feature to automatically resolve discussions when a changeset is present has been properly implemented. The code appropriately checks:
- If a changeset exists
- If auto-resolve is enabled (defaulting to true)
This addresses the PR objectives of helping projects where merge capability depends on resolved discussions.
Verify default value behavior.
The implementation uses
env.GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE || '1'which means it will default to '1' (truthy) if the environment variable is not set. This aligns with the stated default in the changeset.
🏁 Script executed:
#!/bin/bash # Search for any tests or documentation that verify the default behavior rg -A 5 "GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE" --type ts --type mdLength of output: 1253
Auto-resolve feature and default behavior verified
The auto-resolve implementation in
src/comment.ts(lines 309-324) correctly checks for a changeset and uses the environment variableGITLAB_COMMENT_DISCUSSION_AUTO_RESOLVE || '1', ensuring that discussions default to auto-resolve when the variable is unset. The repository’s README and type definitions insrc/types.tsconfirm that this behavior is intentional and aligns with the design objectives.
This PR adds a new optional
GITLAB_COMMENT_DISCUSSION_AUTO_RESOLVEenvironment variable to automatically resolve added discussion when changeset is present, if you want always resolve the discussion, you should actually useGITLAB_COMMENT_TYPE=noteinstead, defaulttrue.This feature should help in projects where ability to merge depends on all resolved discussions. After little internal research all developers appreciate the notification about missing changeset but also this unresolved discussion comment might by useless when changeset check is ok.
close #190
close #191
Summary by CodeRabbit
New Features
Documentation
Refactor