chore/add_tagging_script #62
Conversation
WalkthroughThe Makefile was updated to introduce automated Git tagging for multiple subdirectories using new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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
🧹 Nitpick comments (2)
Makefile (2)
17-19: ExposeSUBDIR_PREFIXESfor caller overrideHard-coding the list reduces flexibility for downstream repos or forks that have additional / fewer sub-modules.
Consider allowing a caller override (default fallback keeps current behaviour):-# Subdirectories for tagging -SUBDIR_PREFIXES := api core errors store x/circuit x/evidence x/feegrant x/nft x/tx x/upgrade +# Sub-directories for tagging (caller can override) +SUBDIR_PREFIXES ?= api core errors store x/circuit x/evidence x/feegrant x/nft x/tx x/upgrade
510-512: Restore the previous branch to avoid leaving users in detached-HEADAfter tagging, contributors remain on a detached commit, which can be confusing:
done @echo "Successfully created and pushed all subdir tags for $(TAG)" + @orig_branch=$$(git symbolic-ref -q --short HEAD 2>/dev/null || echo ""); \ + if [ -n "$$orig_branch" ]; then git checkout "$$orig_branch"; fiSmall UX improvement; no functional downside.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Makefile(2 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 513-513: Missing required phony target "all"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: tests (00)
- GitHub Check: tests (03)
- GitHub Check: tests (02)
- GitHub Check: tests (01)
- GitHub Check: test-sim-nondeterminism
- GitHub Check: test-e2e
- GitHub Check: liveness-test
- GitHub Check: test-integration
- GitHub Check: golangci-lint
- GitHub Check: dependency-review
- GitHub Check: build (amd64)
- GitHub Check: build (arm64)
- GitHub Check: build (arm)
Makefile
Outdated
| tag-subdirs: | ||
| ifndef TAG | ||
| $(error TAG is required. Usage: make tag-subdirs TAG=v1.2.3) | ||
| endif | ||
| @echo "Checking out tag $(TAG) from origin..." | ||
| @git fetch origin | ||
| @git checkout $(TAG) | ||
| @echo "Creating and pushing subdir tags for $(TAG)..." |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate that the supplied TAG exists before checking out
git checkout $(TAG) will fail – and leave make in a detached-HEAD state – if the tag does not exist locally or remotely.
Add a lightweight verification step and fetch only the tag you need:
- @echo "Checking out tag $(TAG) from origin..."
- @git fetch origin
- @git checkout $(TAG)
+ @echo "Verifying tag $(TAG) exists in origin..."
+ @git fetch --quiet origin "refs/tags/$(TAG):refs/tags/$(TAG)" || \
+ ( echo "Tag $(TAG) not found in origin" && exit 1 )
+ @echo "Checking out tag $(TAG)..."
+ @git checkout --detach $(TAG)Benefits: deterministic fetch, clear error, avoids polluting local branches.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tag-subdirs: | |
| ifndef TAG | |
| $(error TAG is required. Usage: make tag-subdirs TAG=v1.2.3) | |
| endif | |
| @echo "Checking out tag $(TAG) from origin..." | |
| @git fetch origin | |
| @git checkout $(TAG) | |
| @echo "Creating and pushing subdir tags for $(TAG)..." | |
| tag-subdirs: | |
| ifndef TAG | |
| $(error TAG is required. Usage: make tag-subdirs TAG=v1.2.3) | |
| endif | |
| @echo "Verifying tag $(TAG) exists in origin..." | |
| @git fetch --quiet origin "refs/tags/$(TAG):refs/tags/$(TAG)" || \ | |
| ( echo "Tag $(TAG) not found in origin" && exit 1 ) | |
| @echo "Checking out tag $(TAG)..." | |
| @git checkout --detach $(TAG) | |
| @echo "Creating and pushing subdir tags for $(TAG)..." |
🤖 Prompt for AI Agents
In the Makefile around lines 496 to 503, before running 'git checkout $(TAG)',
add a verification step to check if the tag exists locally or remotely. Use a
lightweight git command like 'git rev-parse --verify refs/tags/$(TAG)' or 'git
ls-remote --tags origin $(TAG)' to confirm the tag's existence. If the tag does
not exist, output a clear error message and stop the make process. Also, modify
the fetch command to fetch only the specific tag instead of all refs to avoid
unnecessary data and local branch pollution.
| @for prefix in $(SUBDIR_PREFIXES); do \ | ||
| new_tag="$$prefix/$(TAG)"; \ | ||
| echo "Creating tag: $$new_tag"; \ | ||
| git tag "$$new_tag" $(TAG); \ | ||
| echo "Pushing tag: $$new_tag"; \ | ||
| git push origin "$$new_tag"; \ | ||
| done |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle pre-existing sub-dir tags to make the rule idempotent
Running the rule twice currently hard-fails on git tag or git push if the tag already exists.
Guard against it and add simple annotation for traceability:
- new_tag="$$prefix/$(TAG)"; \
- echo "Creating tag: $$new_tag"; \
- git tag "$$new_tag" $(TAG); \
- echo "Pushing tag: $$new_tag"; \
- git push origin "$$new_tag"; \
+ new_tag="$$prefix/$(TAG)"; \
+ if git rev-parse -q --verify "$$new_tag" >/dev/null; then \
+ echo "Tag $$new_tag already exists – skipping"; \
+ else \
+ echo "Creating tag: $$new_tag"; \
+ git tag -a "$$new_tag" -m "Auto-generated subdir tag for $$prefix $(TAG)" $(TAG); \
+ echo "Pushing tag: $$new_tag"; \
+ git push origin "$$new_tag"; \
+ fi; \This keeps the target repeatable and prevents accidental failures in CI.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @for prefix in $(SUBDIR_PREFIXES); do \ | |
| new_tag="$$prefix/$(TAG)"; \ | |
| echo "Creating tag: $$new_tag"; \ | |
| git tag "$$new_tag" $(TAG); \ | |
| echo "Pushing tag: $$new_tag"; \ | |
| git push origin "$$new_tag"; \ | |
| done | |
| @for prefix in $(SUBDIR_PREFIXES); do \ | |
| new_tag="$$prefix/$(TAG)"; \ | |
| if git rev-parse -q --verify "$$new_tag" >/dev/null; then \ | |
| echo "Tag $$new_tag already exists – skipping"; \ | |
| else \ | |
| echo "Creating tag: $$new_tag"; \ | |
| git tag -a "$$new_tag" -m "Auto-generated subdir tag for $$prefix $(TAG)" $(TAG); \ | |
| echo "Pushing tag: $$new_tag"; \ | |
| git push origin "$$new_tag"; \ | |
| fi; \ | |
| done |
🤖 Prompt for AI Agents
In the Makefile around lines 504 to 510, the current tag creation and push
commands fail if the tag already exists, making the rule non-idempotent. Modify
the script to first check if the tag "$$new_tag" already exists using git tag
-l, and only create and push the tag if it does not exist. When creating the
tag, add an annotation with a message for traceability. This will prevent errors
on repeated runs and ensure the rule is repeatable and safe for CI environments.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
518-524: Handle pre-existing sub-dir tags (idempotency)Same concern as previously raised: re-running the target fails when a tag already exists. Add an existence check before
git tag/git push, mirroring the proposed fix fortag-client.
🧹 Nitpick comments (2)
Makefile (2)
17-19: Declare prefixes in a make-safe wayA minor nit: if at some point a path in
SUBDIR_PREFIXEScontains spaces or other shell-special characters, word-splitting will break the loop later on. Wrapping each item in single quotes (or using a newline-separated variable) future-proofs the rule.
512-517: Redundant secondgit fetch
tag-subdirsdepends ontag-client, which already fetches tags.
The extra fetch here adds latency with no benefit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Makefile(2 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 527-527: Missing required phony target "all"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: tests (03)
- GitHub Check: tests (02)
- GitHub Check: tests (01)
- GitHub Check: tests (00)
- GitHub Check: test-e2e
- GitHub Check: test-integration
- GitHub Check: test-sim-nondeterminism
- GitHub Check: liveness-test
- GitHub Check: build (arm)
- GitHub Check: build (amd64)
- GitHub Check: build (arm64)
- GitHub Check: golangci-lint
- GitHub Check: dependency-review
| # tag-client creates and pushes the special client tag with v2.0.0- prefix | ||
| # Usage: make tag-client TAG=v1.2.3 (creates client/v2.0.0-v1.2.3) | ||
| tag-client: | ||
| ifndef TAG | ||
| $(error TAG is required. Usage: make tag-client TAG=v1.2.3) | ||
| endif | ||
| @echo "Fetching latest tags from origin..." | ||
| @git fetch origin --tags | ||
| @echo "Creating client tag for $(TAG)..." | ||
| @client_tag="client/v2.0.0-$(TAG)"; \ | ||
| echo "Creating tag: $$client_tag"; \ | ||
| git tag "$$client_tag" $(TAG); \ | ||
| echo "Pushing tag: $$client_tag"; \ | ||
| git push origin "$$client_tag" | ||
| @echo "Successfully created and pushed client tag for $(TAG)" | ||
|
|
There was a problem hiding this comment.
tag-client is not idempotent and lacks basic validation
Running the target twice hard-fails (git tag / git push), and it will happily create a tag even when $(TAG) is missing locally / remotely.
@@
- @echo "Fetching latest tags from origin..."
- @git fetch origin --tags
- @echo "Creating client tag for $(TAG)..."
- @client_tag="client/v2.0.0-$(TAG)"; \
- echo "Creating tag: $$client_tag"; \
- git tag "$$client_tag" $(TAG); \
- echo "Pushing tag: $$client_tag"; \
- git push origin "$$client_tag"
+ @echo "Verifying tag $(TAG) exists in origin…"
+ @git fetch --quiet origin "refs/tags/$(TAG):refs/tags/$(TAG)" || \
+ ( echo "Tag $(TAG) not found in origin" && exit 1 )
+ @client_tag="client/v2.0.0-$(TAG)"; \
+ if git rev-parse -q --verify "$$client_tag" >/dev/null; then \
+ echo "Tag $$client_tag already exists — skipping"; \
+ else \
+ echo "Creating tag: $$client_tag"; \
+ git tag -a "$$client_tag" -m "Auto-generated client tag for $(TAG)" $(TAG); \
+ echo "Pushing tag: $$client_tag"; \
+ git push origin "$$client_tag"; \
+ fiBenefits: repeatable runs, clear error message, no local-branch pollution, traceable annotated tag.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # tag-client creates and pushes the special client tag with v2.0.0- prefix | |
| # Usage: make tag-client TAG=v1.2.3 (creates client/v2.0.0-v1.2.3) | |
| tag-client: | |
| ifndef TAG | |
| $(error TAG is required. Usage: make tag-client TAG=v1.2.3) | |
| endif | |
| @echo "Fetching latest tags from origin..." | |
| @git fetch origin --tags | |
| @echo "Creating client tag for $(TAG)..." | |
| @client_tag="client/v2.0.0-$(TAG)"; \ | |
| echo "Creating tag: $$client_tag"; \ | |
| git tag "$$client_tag" $(TAG); \ | |
| echo "Pushing tag: $$client_tag"; \ | |
| git push origin "$$client_tag" | |
| @echo "Successfully created and pushed client tag for $(TAG)" | |
| # tag-client creates and pushes the special client tag with v2.0.0- prefix | |
| # Usage: make tag-client TAG=v1.2.3 (creates client/v2.0.0-v1.2.3) | |
| tag-client: | |
| ifndef TAG | |
| $(error TAG is required. Usage: make tag-client TAG=v1.2.3) | |
| endif | |
| @echo "Verifying tag $(TAG) exists in origin…" | |
| @git fetch --quiet origin "refs/tags/$(TAG):refs/tags/$(TAG)" || \ | |
| ( echo "Tag $(TAG) not found in origin" && exit 1 ) | |
| @client_tag="client/v2.0.0-$(TAG)"; \ | |
| if git rev-parse -q --verify "$$client_tag" >/dev/null; then \ | |
| echo "Tag $$client_tag already exists — skipping"; \ | |
| else \ | |
| echo "Creating tag: $$client_tag"; \ | |
| git tag -a "$$client_tag" -m "Auto-generated client tag for $(TAG)" $(TAG); \ | |
| echo "Pushing tag: $$client_tag"; \ | |
| git push origin "$$client_tag"; \ | |
| fi | |
| @echo "Successfully created and pushed client tag for $(TAG)" |
🤖 Prompt for AI Agents
In the Makefile from lines 492 to 507, the tag-client target is not idempotent
and lacks validation, causing failures if run multiple times or if the TAG does
not exist locally or remotely. To fix this, add checks to verify that the TAG
exists locally before creating the client tag, and check if the client tag
already exists to avoid duplicate creation. Use annotated tags instead of
lightweight tags for traceability, and handle errors gracefully with clear
messages. Also, ensure the tag push only happens if the tag creation succeeds,
making the process repeatable and safe.
Summary by CodeRabbit
New Features
Chores