Support conventional commits #484
Conversation
68b4928 to
bc54e9d
Compare
|
Caution Review failedThe pull request is closed. WalkthroughThe changes add a new input parameter ( Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Action Runner
participant Context as getInputs (src/context.ts)
participant Notes as groupDependencyUpdates (src/notes.ts)
Runner->>Context: Provide inputs (including remove-conventional-prefixes)
Context-->>Runner: Return Inputs object with removeConventionalPrefixes flag
Runner->>Notes: Call groupDependencyUpdates(sections, {removeConventionalPrefixes})
alt Flag enabled
Notes->>Notes: Remove conventional prefixes and format entries
else Flag disabled
Notes->>Notes: Process entries without modifications
end
Notes-->>Runner: Return processed dependency update entries
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (7)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
✅ Actions performedFull review triggered. |
1 similar comment
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
__tests__/release.test.ts (1)
151-151: Add a targeted test case forremoveConventionalPrefixes.Currently,
removeConventionalPrefixes: falseis set here without verifying its effect. To ensure full coverage, consider adding a test that simulates a scenario where the release notes would contain conventional commit prefixes and asserts that they remain intact.__tests__/notes.test.ts (2)
128-128: Extend test scenarios to confirm prefix retention.Like the previous instance, this reinforces that the default behavior is to keep conventional prefixes. Consider adding more specialized assertions or input variations to confirm correctness under different conditions.
621-638: Assert the default prefix behavior.This test complements the previous one by confirming that prefixes remain when
removeConventionalPrefixesis disabled. The coverage is solid, but consider also verifying that multiple identical dependencies remain grouped appropriately regardless of prefix presence.src/notes.ts (1)
344-347: Lock file maintenance prefix condition.Conditionally injecting the prefix is a clean approach, consistent with the other patterns. If you foresee more specialized behaviors, consider extracting it into a helper function for easier maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
dist/context.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/notes.d.tsis excluded by!**/dist/**
📒 Files selected for processing (7)
README.md(1 hunks)__tests__/notes.test.ts(4 hunks)__tests__/release.test.ts(1 hunks)__tests__/version.test.ts(1 hunks)action.yml(1 hunks)src/context.ts(2 hunks)src/notes.ts(6 hunks)
🔇 Additional comments (19)
__tests__/version.test.ts (1)
18-18: Change looks good - adds support for the new input parameterThe addition of
removeConventionalPrefixes: falseto thefakeInputsobject is appropriate and maintains consistency with the interface changes insrc/context.ts.action.yml (1)
37-40: Clearly documents the new conventional commit prefixes featureThe new input parameter is well-documented with a clear description and appropriate default value. The placement in the configuration file is logical, following other formatting-related options like
group-dependencies.README.md (1)
40-53: Improved table formatting and documentation of the new featureThe markdown table has been reformatted for better readability, and the new
remove-conventional-prefixesparameter is properly documented with consistent formatting. The description clearly explains its purpose in removing conventional prefixes from PR titles.src/context.ts (2)
16-16: Good addition to the Inputs interfaceThe new
removeConventionalPrefixesproperty is properly added to theInputsinterface. The naming is consistent with the kebab-case to camelCase conversion pattern used for other input parameters.
42-42: Implementation matches interface and action.yml definitionThe
getInputsfunction is properly updated to retrieve the boolean value usingcore.getBooleanInput, consistent with how other boolean inputs are handled.__tests__/notes.test.ts (7)
87-87: Well-defined default behavior.Using
removeConventionalPrefixes: falsehere is consistent with preserving prefixes by default. This aligns with the PR objective of making the prefix removal an opt-in feature.
496-496: Check grouping impact.This line introduces a second fix entry for the same dependency. Ensure that the downstream grouping logic correctly merges this entry when
removeConventionalPrefixesis toggled, especially since it’s a separate PR ID.
505-505: Ensure consistent usage of the options object.You are explicitly passing
{ removeConventionalPrefixes: true }here. Verify that callinggroupDependencyUpdatesin other places leverages the same interface consistently and that the tests cover both true and false scenarios extensively.
509-512: Succinctly verifying merges in test results.You are checking that multiple PRs for the same dependency are merged into a single line. The logic looks correct. Great job ensuring the test coverage for both DependaBot and Renovate.
597-619: Good coverage for removing conventional commit prefixes.This test effectively validates the removal of
fix(deps):,chore(deps):, etc. from the final grouped entries. The test scenario is thorough, covering multiple prefix cases in a single group.
640-659: Helpful test for verifying group integrity post-prefix removal.Here, you showcase that removing the prefixes does not compromise accurate grouping for multiple dependencies. The logic and test are well-aligned—nice work.
661-681: Broad coverage for advanced prefix variants.This test effectively handles multiple advanced prefix forms (
ci,build,chore(deps-dev), etc.). The step-by-step grouping is well-validated here.src/notes.ts (7)
69-71: Conditional grouping logic.By conditionally invoking
groupDependencyUpdateswithremoveConventionalPrefixes, you allow flexible release note formatting. Confirm that any conditionally updated sections still match the user’s natural reading expectation.
301-306: EnhancedgroupDependencyUpdatessignature.Accepting the new
optionsobject withremoveConventionalPrefixesincreases this function’s extensibility. Good approach for possible future additions.
308-316: Well-structured prefix handling.Defining the
conventionalPrefixRegexandoptionalPrefixPatternensures the prefix removal logic is central and consistent. Great job maintaining readability.
393-393: Prefix state in grouping.Storing
prefixin the grouping logic is a subtle but crucial step toward re-applying the original text ifremoveConventionalPrefixesis false. This maintains consistent representation of each entry.
401-412: Robust prefix removal before grouping.This ensures the item is updated in-place, guaranteeing consistent grouping keys. Verify that no unintended transformations occur for lines lacking a recognized prefix.
426-431: Conditional prefix extraction.When
removeConventionalPrefixesis disabled, the logic elegantly extracts the prefix from the matched line. This design fosters clarity between the with/without prefix flows.
650-657: Helper for capitalizing first word.This utility is straightforward and exactly what's needed to tidy up the first word after prefix removal. Thoroughly tested in the new test cases—nicely done.
5b29ef6 to
93545c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
__tests__/notes.test.ts (4)
87-87: Consider adding a test case withremoveConventionalPrefixes: true.Here, you set the default to
false, which is consistent with your summaries. Nonetheless, testing at least one scenario withtruein this same suite ensures robust coverage without relying solely on separate tests.
128-128: Duplicated default configuration.Same observation—you're setting
removeConventionalPrefixes: falseagain. Ensure the suite also covers scenarios whereremoveConventionalPrefixesistrue(or confirm they’re tested in other blocks).
597-624: Test coverage for mixed prefix types is comprehensive.Your test accurately checks prefix precedence between older and newer versions. Consider adding a corner case where the dependency name differs slightly (e.g., capitalized vs. lowercase) to ensure consistent grouping under all naming variations.
646-753: Extensive test suite forremoveConventionalPrefixes.These tests thoroughly cover various prefix cases, empties, and capitalized words. Minor suggestion: adjust the test name “capitalized first word after removing prefix” to a more consistent style like “should capitalize the first word after removing prefix.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
dist/context.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/notes.d.tsis excluded by!**/dist/**
📒 Files selected for processing (7)
README.md(1 hunks)__tests__/notes.test.ts(6 hunks)__tests__/release.test.ts(1 hunks)__tests__/version.test.ts(1 hunks)action.yml(1 hunks)src/context.ts(2 hunks)src/notes.ts(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/version.test.ts
- action.yml
- tests/release.test.ts
- src/context.ts
- README.md
- src/notes.ts
🔇 Additional comments (4)
__tests__/notes.test.ts (4)
2-2: Addition ofremoveConventionalPrefixesimport looks solid.You're now importing
removeConventionalPrefixes, aligning with your new feature tests. This is a straightforward and correct addition.
496-496: Good job including a lower version prefix example.This test entry helps verify proper grouping logic when multiple versions exist for the same dependency. No further changes needed.
509-509: Correct grouping outcome.The merged PR references show that the logic for combining repeated dependencies is functioning as intended. Nice work.
626-644: Robust validation of “special format” commit prefixes.The scenario coverage for unconventional prefixes (e.g.,
refactor(deps)orperf(deps)) looks good. The test ensures the output remains unchanged whenremoveConventionalPrefixesisfalse.
93545c8 to
50ea4c2
Compare
There was a problem hiding this comment.
PR Overview
This PR adds support for conventional commits by introducing a new configuration option to remove conventional commit prefixes from dependency update messages. The key changes include updating test cases, modifying the main release note generation logic in src/notes.ts to support prefix removal, and adding the corresponding configuration in both the action and documentation.
Reviewed Changes
| File | Description |
|---|---|
| tests/notes.test.ts | Updated tests to verify behavior when removing conventional commit prefixes. |
| src/notes.ts | Added support for removing conventional commit prefixes and updated dependency grouping logic. |
| src/context.ts | Extended the Inputs interface to include the new removeConventionalPrefixes option. |
| action.yml | Added a new input for remove-conventional-prefixes with default set to false. |
| README.md | Updated documentation to describe the new remove-conventional-prefixes input. |
| tests/version.test.ts | Updated tests to pass the new removeConventionalPrefixes input. |
| tests/release.test.ts | Updated tests to pass the new removeConventionalPrefixes input. |
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
| * @param regex - The regex pattern to use for matching the prefix. | ||
| * @returns The extracted prefix including scope and colon+space, or an empty string if none found. | ||
| */ | ||
| function getPrefix(str: string, regex: RegExp): string { |
There was a problem hiding this comment.
[nitpick] Ensure that getPrefix correctly handles cases with extra whitespace between the bullet point and the conventional prefix. Consider using a more robust trimming strategy rather than relying solely on substring(2).
Copilot uses AI. Check for mistakes.
Closes #302
Summary by CodeRabbit