Skip to content

Allow configuring the rolling major#323

Open
cole-h wants to merge 2 commits into
mainfrom
flk-391
Open

Allow configuring the rolling major#323
cole-h wants to merge 2 commits into
mainfrom
flk-391

Conversation

@cole-h

@cole-h cole-h commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features
    • Added optional rolling-major configuration so rolling releases can use a custom major.minor.commit-count versioning instead of a fixed leading major.
    • CLI/environment and action inputs now accept and propagate the rolling-major value when provided.
  • Documentation
    • Updated parameter docs and descriptions to show the new rolling-major option and the revised version format "[rolling-major].[rolling-minor].[commit count]+rev-[git sha]".

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bc6db6b7-345a-4b13-828f-cbca29a01c50

📥 Commits

Reviewing files that changed from the base of the PR and between 44b171a and 6ac6b5c.

⛔ Files ignored due to path filters (2)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (4)
  • README.md
  • action.yaml
  • src/cli/mod.rs
  • ts/index.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • ts/index.ts
  • action.yaml
  • src/cli/mod.rs

📝 Walkthrough

Walkthrough

Adds an optional rolling-major input and threads it from action.yaml through ts/index.ts into src/cli/mod.rs, where release_version now composes {major}.{minor} (or 0.{minor}) and appends .{commit_count}+rev-{git sha} for rolling releases.

Changes

Rolling-major parameter and versioning

Layer / File(s) Summary
Input documentation and contract
action.yaml, README.md
Adds the rolling-major input and updates rolling parameter docs to describe [rolling-major].[rolling-minor].[commit count]+rev-[git sha].
TypeScript GitHub Actions integration
ts/index.ts
Adds rollingMajor property, reads rolling-major via inputs.getNumberOrNull, extends ExecutionEnvironment with FLAKEHUB_PUSH_ROLLING_MAJOR, and conditionally sets the env var.
CLI argument parsing for rolling-major
src/cli/mod.rs
Adds rolling_major: Option<u64> to FlakeHubPushCli, wired to --rolling-major and FLAKEHUB_PUSH_ROLLING_MAJOR using U64ToNoneParser.
Release version computation with rolling-major
src/cli/mod.rs
release_version now considers rolling_major, rolling_minor, and tag together, enforces validation (e.g., rolling-major requires rolling-minor), selects {major}.{minor} or 0.{minor}, and appends .{commit_count}+rev-{revision} when any rolling component is set.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I nibble on code with cheer,
Major rolling now appears.
From input, env, to CLI's art,
Versions hop forward — neat from start.
A tiny patch, a happy heart. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding the ability to configure the rolling major version component, which is the primary feature introduced across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch flk-391

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
action.yaml (1)

47-52: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update rolling input description to reflect configurable major.

Line 51 still documents the old fixed-major scheme (0.1...) and only mentions rolling-minor, which now conflicts with the new rolling-major input docs.

Suggested doc fix
   rolling:
     description: |
       For untagged releases, use a rolling versioning scheme.

-      When this is enabled, the default versioning scheme is 0.1.[commit count]+rev-[git sha]. To customize the SemVer minor version, set the `rolling-minor` option.
+      When this is enabled, the default versioning scheme is 0.1.[commit count]+rev-[git sha]. To customize the SemVer major/minor version, set `rolling-major` and `rolling-minor`.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@action.yaml` around lines 47 - 52, The rolling input's description in
action.yaml is out of date — it still documents a fixed major ("0.1...") and
only mentions rolling-minor while a new rolling-major option exists; update the
rolling input description to describe the new configurable major and minor
scheme (e.g., "0.{rolling-major}.{rolling-minor}[commit count]+rev-[git sha]" or
similar), mention both rolling-major and rolling-minor inputs and how they
influence the generated version, and remove the outdated "0.1..." wording so the
rolling description accurately reflects the current behavior of the rolling
option.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/mod.rs`:
- Around line 415-418: Update the validation error in the branch that checks
(Some(_), Some(_), _) if !self.rolling so the message mentions both
rolling-major and rolling-minor; specifically, in the function/method where the
flags self.rolling, rolling-major and rolling-minor are validated (the match arm
currently returning Err(eyre!(...))), change the error text to say you must
enable `rolling` to upload a release when both `rolling-major` and
`rolling-minor` are set (e.g. "You must enable `rolling` to upload a release
when both `rolling-major` and `rolling-minor` are specified.").

---

Outside diff comments:
In `@action.yaml`:
- Around line 47-52: The rolling input's description in action.yaml is out of
date — it still documents a fixed major ("0.1...") and only mentions
rolling-minor while a new rolling-major option exists; update the rolling input
description to describe the new configurable major and minor scheme (e.g.,
"0.{rolling-major}.{rolling-minor}[commit count]+rev-[git sha]" or similar),
mention both rolling-major and rolling-minor inputs and how they influence the
generated version, and remove the outdated "0.1..." wording so the rolling
description accurately reflects the current behavior of the rolling option.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4a4ce826-d319-4562-a53b-139afe00b58c

📥 Commits

Reviewing files that changed from the base of the PR and between 3135d10 and e881261.

⛔ Files ignored due to path filters (2)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (3)
  • action.yaml
  • src/cli/mod.rs
  • ts/index.ts

Comment thread src/cli/mod.rs Outdated
@cole-h cole-h marked this pull request as ready for review June 9, 2026 21:46
@cole-h cole-h requested review from colemickens and grahamc June 9, 2026 21:46

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/cli/mod.rs (1)

421-421: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing validation: both rolling-major and rolling-minor set without rolling.

Line 421 matches when both rolling_major and rolling_minor are set, but it has no guard requiring self.rolling to be true. This means users can set both rolling version components without enabling the rolling flag, which contradicts the validation logic on line 415 that requires rolling when either component is set individually.

The postfix logic at line 444 will append rolling-style suffixes (.{commit_count}+rev-{revision}) because it checks self.rolling_major.0.is_some(), resulting in a rolling version even though the user didn't enable rolling.

🛡️ Add validation guard
             (None, Some(minor), _) => format!("0.{minor}"),
-            (Some(major), Some(minor), _) => format!("{major}.{minor}"),
+            (Some(_), Some(_), _) if !self.rolling => {
+                return Err(eyre!(
+                    "You must enable `rolling` to upload a release with a specific `rolling-major` and `rolling-minor`."
+                ));
+            }
+            (Some(major), Some(minor), _) => format!("{major}.{minor}"),
             (None, None, _) if self.rolling => DEFAULT_ROLLING_PREFIX.to_string(),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/mod.rs` at line 421, When both rolling_major and rolling_minor are
Some but self.rolling is not enabled the current match arm (Some(major),
Some(minor), _) produces a rolling-style base version incorrectly; update the
validation/branch that formats the base version (the match handling
rolling_major / rolling_minor) to require self.rolling to be true before
treating paired components as a rolling version (i.e., add a guard checking
self.rolling or move this case behind a check like if self.rolling), and ensure
the postfix logic that inspects rolling_major/rolling_minor also respects
self.rolling so the rolling suffix (.{commit_count}+rev-{revision}) is only
appended when self.rolling is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/cli/mod.rs`:
- Line 421: When both rolling_major and rolling_minor are Some but self.rolling
is not enabled the current match arm (Some(major), Some(minor), _) produces a
rolling-style base version incorrectly; update the validation/branch that
formats the base version (the match handling rolling_major / rolling_minor) to
require self.rolling to be true before treating paired components as a rolling
version (i.e., add a guard checking self.rolling or move this case behind a
check like if self.rolling), and ensure the postfix logic that inspects
rolling_major/rolling_minor also respects self.rolling so the rolling suffix
(.{commit_count}+rev-{revision}) is only appended when self.rolling is true.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c5ed669-f986-4ee8-bfa1-46e951fbf014

📥 Commits

Reviewing files that changed from the base of the PR and between e881261 and 581b006.

⛔ Files ignored due to path filters (2)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (1)
  • src/cli/mod.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@action.yaml`:
- Around line 39-45: Update the input description for the "rolling-major" action
input to document its prerequisites: state that "rolling-major" can only be used
when the "rolling" flag is true and that "rolling-minor" must also be provided
when "rolling-major" is set; reference the input names "rolling-major" and
"rolling-minor" in the description so users see the dependency and avoid invalid
combinations at runtime.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9f2b0e3-57f9-4479-b657-a27bf87735eb

📥 Commits

Reviewing files that changed from the base of the PR and between 581b006 and 44b171a.

⛔ Files ignored due to path filters (2)
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (4)
  • README.md
  • action.yaml
  • src/cli/mod.rs
  • ts/index.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • ts/index.ts
  • src/cli/mod.rs

Comment thread action.yaml
Comment on lines +39 to 45
rolling-major:
description: "Specify the SemVer major version of your rolling releases. All releases will follow the versioning scheme '[rolling-major].[rolling-minor].[commit count]+rev-[git sha]'"
required: false
default: null
rolling-minor:
description: "Specify the SemVer minor version of your rolling releases. All releases will follow the versioning scheme '0.[rolling-minor].[commit count]+rev-[git sha]'"
description: "Specify the SemVer minor version of your rolling releases. All releases will follow the versioning scheme '[rolling-major].[rolling-minor].[commit count]+rev-[git sha]'"
required: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Document rolling-major prerequisites to avoid invalid input combinations.

rolling-major is currently documented as standalone, but downstream CLI validation requires rolling=true and also requires rolling-minor when rolling-major is set. Please include that constraint in this input description to prevent avoidable action failures at runtime (e.g., “rolling-major requires rolling and rolling-minor”).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@action.yaml` around lines 39 - 45, Update the input description for the
"rolling-major" action input to document its prerequisites: state that
"rolling-major" can only be used when the "rolling" flag is true and that
"rolling-minor" must also be provided when "rolling-major" is set; reference the
input names "rolling-major" and "rolling-minor" in the description so users see
the dependency and avoid invalid combinations at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant