-
Notifications
You must be signed in to change notification settings - Fork 7
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
Proper version names #347
Proper version names #347
Conversation
WalkthroughThe pull request introduces changes across multiple files to modify how commit identifiers are generated and utilized. Key modifications include renaming a job in a workflow and adjusting the format of commit hashes to include a "dev-" prefix while retaining the full hashes. These changes affect GitHub workflow configurations for building Docker images and development run scripts, ensuring a consistent approach to versioning and commit identification throughout the project. Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Workflow as GitHub Workflow
participant Docker as Docker Build
Dev->>Workflow: Trigger build
Workflow->>Workflow: Generate commit identifier with dev- prefix
Workflow->>Docker: Build image with dev-{commitID}
Docker-->>Workflow: Image built
Workflow-->>Dev: Build complete
The sequence diagram illustrates the workflow for generating and using commit identifiers, emphasizing the new "dev-" prefix across different build processes. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 (
|
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
🧹 Nitpick comments (1)
.github/workflows/release-from-tag.yml (1)
58-58
: LGTM! Appropriate version naming for releases.The format
<tag/ref>-<commit_sha>
is appropriate for release builds, correctly differentiating them from development builds that use thedev-
prefix.Add a newline at the end of the file to fix the YAML formatting issue.
build-args: "GIT_COMMIT=${{ github.ref_name }}-${{ github.event_name != 'pull_request' && github.sha || github.event.pull_request.head.sha }}" +
🧰 Tools
🪛 yamllint (1.35.1)
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build-pre-baked-images.yml
(1 hunks).github/workflows/build-xmtpd.yml
(2 hunks).github/workflows/release-from-tag.yml
(1 hunks)dev/run
(1 hunks)dev/run-2
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/build-pre-baked-images.yml
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/release-from-tag.yml
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
dev/run-2 (1)
15-15
: LGTM! Consistent with dev/run implementation.
The version naming pattern matches the standardization implemented in dev/run.
.github/workflows/build-xmtpd.yml (2)
56-57
: LGTM! Good practice using a pinned version.
The addition of the short-sha action is well-structured and uses a specific version for stability.
67-67
: Verify the impact of the new version format.
The standardization of commit identifiers with the "dev-" prefix and short SHA is good. However, let's verify that this change is consistent with other components that might consume this build argument.
✅ Verification successful
Based on the search results, I can generate the final response since I have enough information to verify the impact of the version format change:
The "dev-" prefix format is safe to use and consistent with the codebase
The version format change to use "dev-" prefix with short SHA is safe because:
- The GIT_COMMIT build arg is only used in two places:
dev/docker/Dockerfile
: Used with-ldflags
to embed commit info intoxmtpd
binarydev/docker/Dockerfile-cli
: Used with-ldflags
to embed commit info intoxmtpd-cli
binary
- The commit value is only used for version reporting through the
-v/--version
flags in the CLI, as seen incmd/cli/main.go
- There are no version parsing or validation logic in the codebase that depends on the specific format of this commit identifier
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other occurrences of GIT_COMMIT usage to ensure compatibility
# Search for GIT_COMMIT references in Dockerfiles and scripts
echo "Checking Dockerfiles and scripts for GIT_COMMIT usage:"
rg -A 3 "GIT_COMMIT" dev/docker/
# Search for version/commit parsing in the codebase
echo "Checking for version/commit parsing logic:"
rg -A 3 "dev-[a-f0-9]|version|commit" --type go
Length of output: 38545
dev/run
Outdated
go run -ldflags="-X main.Commit=dev-$(git rev-parse --short HEAD)" cmd/replication/main.go "$@" |
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.
💡 Codebase verification
Inconsistent version naming patterns found in development scripts
The codebase shows inconsistent version naming patterns:
dev/run
anddev/run-2
usedev-$(git rev-parse --short HEAD)
dev/cli
uses full commit hash without prefix:$(git rev-parse HEAD)
- Docker-related files use
GIT_COMMIT
with full hash
Locations needing attention:
dev/cli
: Update to use short hash withdev-
prefixdev/docker/build
: Consider using short hashdev/docker/Dockerfile
anddev/docker/Dockerfile-cli
: Consider standardizing version format
🔗 Analysis chain
LGTM! Verify consistent version naming across the codebase.
The change to use dev-$(git rev-parse --short HEAD)
aligns with the standardization of version names.
Let's verify the consistency of version naming across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other potential version/commit identifier patterns
rg -g '!*.md' -g '!*.json' -g '!*.lock' '(?:main\.Commit=|-X main\.Commit|\bGIT_COMMIT\b)'
Length of output: 826
Summary by CodeRabbit
New Features
GIT_COMMIT
argument to include tag or branch names.Bug Fixes
Chores