(HOLD) feat: add tagged stable build system for macOS releases #698
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implements a comprehensive tagged stable build system that creates official releases triggered by git tags or manual workflow dispatch. This complements the existing nightly build system by providing stable, versioned releases with semantic versioning. Key changes: - Add git tag push trigger for stable builds (v0.2.0 format) - Implement version detection using GitHub Script action - Auto-increment patch version on manual dispatch - Explicitly set LDFLAGS for stable builds (standard paths/ports) - Proper DMG naming (CodeLayer-darwin-arm64.dmg for stable) - Mark stable releases as latest (not pre-release) - Automatic homebrew cask updates for stable releases The system now supports three build types: 1. Nightly: Automated daily builds with isolated configs 2. Stable: Tag-triggered builds with standard configurations 3. Manual: Dispatch with optional version or auto-increment This allows users to install stable releases via: brew install --cask codelayer While nightly users continue with: brew install --cask codelayer-nightly Both versions can coexist without conflicts.
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to c1b49c4 in 1 minute and 43 seconds. Click for details.
- Reviewed
464lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/release-macos.yml:48
- Draft comment:
When checking the nightly build input, compare as a boolean rather than a string. Since the input 'release_nightly' is defined as a boolean, use=== trueinstead of=== 'true'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While the input is defined as boolean type, GitHub Actions actually provides all inputs as strings. The current code comparing to 'true' is correct. The suggested change would actually break the functionality since comparing a string to boolean with === would always return false. Am I certain about how GitHub Actions handles boolean inputs? Could there be some automatic type conversion I'm not aware of? Yes, this is well documented behavior - GitHub Actions workflow inputs are always provided as strings, regardless of their type definition. The current code comparing to 'true' is the standard pattern used in Actions workflows. The comment should be deleted as it suggests a change that would break the functionality. The current string comparison is correct.
2. .github/workflows/release-macos.yml:82
- Draft comment:
The API call to list tags is limited to 100 results. If the repository has more than 100 tags, the latest stable tag might be missed. Consider handling pagination for accuracy. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a real issue - if the repo has >100 tags, we could miss newer tags and incorrectly calculate the next version number. However, this is a new project (starting at v0.2.0) and unlikely to have 100+ tags soon. The comment is speculative ("might be missed") and suggests something to "consider" rather than demanding a clear fix. The issue could become real in the future as the project grows. Missing the latest tag would cause version numbering issues. While valid, the issue is not currently affecting the project and the comment is speculative. Our rules say not to make speculative comments or suggestions to "consider" things. Delete the comment. While technically correct, it's a speculative "consider this" type comment about a potential future issue, which violates our commenting rules.
3. .github/workflows/release-macos.yml:292
- Draft comment:
The conditional expression for setting the release tag usingformat('v{0}', ...)may be error-prone. Consider using direct string interpolation (e.g.,v${{ steps.version.outputs.release_version }}) for better clarity and reliability. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% Both approaches (format vs concatenation) would produce the same result. The format approach is actually quite standard in many contexts. While concatenation might be slightly more readable here, the difference is minimal and this is really just a style preference. The current code works correctly and the suggested change doesn't fix any actual problems. The suggestion could make the code marginally more readable. However, format() is a perfectly valid approach and is often preferred in other contexts for its flexibility. While the suggestion might offer slightly better readability, the improvement is too minor to justify a required code change. The current code is perfectly functional and clear enough. Delete this comment as it suggests a change that is purely stylistic and doesn't meaningfully improve the code.
Workflow ID: wflow_4XHNbzemseB7xIPK
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
.github/workflows/release-macos.yml
Outdated
| // Create and push the new tag | ||
| try { | ||
| execSync(`git config user.name "github-actions[bot]"`); |
Contributor
There was a problem hiding this comment.
Auto-increment tag creation via execSync is performed before checking out the repository. This can cause git commands (config, tag, push) to fail. Consider moving these commands after a checkout step or including a checkout within this step.
The version generation step was trying to create git tags before the repository was checked out, causing 'not in a git directory' error. Split the logic: - Generate version calculates what tag to create - After checkout, create and push the tag if needed
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 7313b2a in 7 minutes and 11 seconds. Click for details.
- Reviewed
471lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/release-macos.yml:46
- Draft comment:
When checking for nightly builds, you comparerelease_nightlyagainst the string 'true'. Since workflow inputs are strings, this works, but adding a comment to clarify this behavior might help future maintainers. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
2. .github/workflows/release-macos.yml:80
- Draft comment:
The auto-increment logic fetches only 100 tags. If the repo has more than 100 tags, older stable tags might be missed. Consider handling pagination for completeness. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The code is looking for the latest stable tag to increment its patch version. Since tags are sorted by version number, and we only need the most recent one, missing older tags is not actually a problem. The first 100 tags will include the most recent ones, which is all we need for this use case. The comment is technically correct but raises a non-issue. What if there are many pre-release tags that push the latest stable tag beyond the 100 limit? That could cause the wrong version to be selected. The code filters for stable tags first (line 90), so even with many pre-release tags, we'll still get the latest stable tag as long as it's within the last 100 tags total. The risk is very low. While technically correct, this comment points out a theoretical edge case that is extremely unlikely to cause issues in practice, since we only need the most recent stable tag.
3. .github/workflows/release-macos.yml:290
- Draft comment:
The use of theformat()function in the workflow expression fortag_namemay not be universally supported. Consider using string concatenation (e.g. 'v${{ steps.version.outputs.release_version }}') for better compatibility. - 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% The format() function is actually a standard feature in GitHub Actions expressions and is well documented. The suggestion to use string concatenation instead doesn't provide any real benefit or fix any actual compatibility issues. The current code is working as intended and follows GitHub Actions best practices. Could there be some edge cases or specific GitHub Actions runner versions where format() behaves differently? Could string concatenation be more readable? The format() function is a core feature of GitHub Actions expressions, well-tested across all runner versions. The current code is actually more readable as it clearly shows the intention to format a version string. This comment should be deleted as it suggests changing working code without providing any real benefit or fixing any actual issues.
4. .github/workflows/release-macos.yml:225
- Draft comment:
There seems to be a potential typo: the working directory is set to 'hlyr' while the build is executed in 'hld'. Please verify if 'hlyr' is intentional or if it should be 'hld' for consistency. - 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.
Workflow ID: wflow_h0RWDnQgUPOesjBk
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Problem
Currently, we only have nightly builds. We need a stable build system that:
Solution
Extended the existing
release-macos.ymlworkflow to handle three trigger types:Key Changes
Test plan
brew install --cask codelayerResolves ENG-2271
Important
Enhances macOS release workflow to support tagged stable builds, semantic versioning, and automatic homebrew cask updates.
release-macos.ymlto support stable builds triggered by git tags and manual dispatch.codelayer.rb) and nightly (codelayer-nightly.rb) builds.This description was created by
for 7313b2a. You can customize this summary. It will automatically update as commits are pushed.