Leon/rename to dnstapir prefix #47
Conversation
WalkthroughRenames project/artifacts from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as dnstapir-cli
participant Root as cmd.Root
participant Sub as Subcommand
participant System as System (files/services)
User->>CLI: run dnstapir-cli <args>
CLI->>Root: cmd.Execute()
Root->>Sub: dispatch selected subcommand
alt enroll / renew flows
Sub->>System: perform enroll/renew actions (keys, certs, systemd)
note right of System #dff0d8: interacts with /usr/bin/dnstapir-cli, systemd units, packaging paths
else api / filter operations
Sub->>System: perform API/filterlist ops
note right of System #f7f7d9: network/config interactions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zluudg
force-pushed
the
leon/rename-to-dnstapir-prefix
branch
from
bcd307c to
e2665cb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
.gitignore(1 hunks)Makefile(2 hunks)go.mod(1 hunks)main.go(1 hunks)rpm/SPECS/dnstapir-cli.spec(2 hunks)tapir-cli.sample.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- tapir-cli.sample.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- main.go
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Target "PROG" should be declared PHONY.
(phonydeclared)
[warning] 13-13: Target "SPECFILE" should be declared PHONY.
(phonydeclared)
🪛 RuboCop (1.81.1)
rpm/SPECS/dnstapir-cli.spec
[fatal] 1-1: unexpected token tCOLON
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)
(Lint/Syntax)
🔇 Additional comments (6)
go.mod (1)
8-8: Dependency version updated.The
github.com/dnstapir/tapirdependency has been updated to a newer version. This appears to be a necessary companion change to the rename.Makefile (2)
1-1: LGTM: Program name variable updated.The
PROGvariable has been correctly renamed todnstapir-cli, consistent with the project rename.
13-13: LGTM: Spec file path updated.The
SPECFILEvariable has been correctly updated to reference the renamed spec file.rpm/SPECS/dnstapir-cli.spec (3)
1-1: LGTM: Package name updated.The RPM package name has been correctly renamed to
dnstapir-cli.
45-45: Config file path updated consistently.The configuration file path has been correctly updated to
dnstapir-cli.yaml, maintaining consistency with the project rename.
10-11: No issues found. The renamed systemd unit files exist and are correctly referenced.The verification confirms that both
dnstapir-renew.serviceanddnstapir-renew.timerfiles exist atrpm/SOURCES/, and the oldtapir-renew.*files have been properly removed. The spec file references at lines 10-11 and 46-47 are correct.
zluudg
force-pushed
the
leon/rename-to-dnstapir-prefix
branch
from
e2665cb to
2512ea1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
rpm/SPECS/dnstapir-cli.spec (2)
6-6: Fix capitalization: "Cli" should be "CLI".The Summary line uses "Cli" which should be "CLI" (all uppercase) as it's an acronym for "Command Line Interface".
Apply this diff:
-Summary: DNS TAPIR EDGE Cli Tool +Summary: DNS TAPIR EDGE CLI Tool
16-16: Fix capitalization: "ClI" should be "CLI".The Description line uses "ClI" which should be "CLI" (all uppercase) as it's an acronym.
Apply this diff:
-DNS TAPIR EDGE ClI Tool for managing an EDGE deployment +DNS TAPIR EDGE CLI Tool for managing an EDGE deployment
🧹 Nitpick comments (1)
cmd/root.go (1)
151-151: Optional: Update commented reference for completeness.The commented-out code on Line 151 still references the old name
"tapir-cli". While this is low priority since it's commented out, you may want to update it to"dnstapir-cli"for consistency.Apply this diff if you'd like to update the comment:
- // GlobalCF.Caller = "tapir-cli" + // GlobalCF.Caller = "dnstapir-cli"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
.gitignore(1 hunks)Makefile(2 hunks)cmd/root.go(2 hunks)go.mod(1 hunks)main.go(1 hunks)rpm/SOURCES/dnstapir-renew.service(1 hunks)rpm/SOURCES/dnstapir-renew.timer(1 hunks)rpm/SPECS/dnstapir-cli.spec(2 hunks)tapir-cli.sample.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- tapir-cli.sample.yaml
✅ Files skipped from review due to trivial changes (1)
- rpm/SOURCES/dnstapir-renew.service
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Target "PROG" should be declared PHONY.
(phonydeclared)
[warning] 13-13: Target "SPECFILE" should be declared PHONY.
(phonydeclared)
🪛 RuboCop (1.81.1)
rpm/SPECS/dnstapir-cli.spec
[fatal] 1-1: unexpected token tCOLON
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)
(Lint/Syntax)
🔇 Additional comments (14)
.gitignore (1)
29-29: LGTM! Binary name added to gitignore.The new binary name
dnstapir-clihas been correctly added to the ignore list, alongside the old name for backward compatibility.go.mod (2)
1-1: LGTM! Module path renamed consistently.The module path has been updated from
tapir-clitodnstapir-cli, aligning with the project rename.
8-8: LGTM! Dependency updated.The
github.com/dnstapir/tapirdependency has been bumped to a newer version.rpm/SOURCES/dnstapir-renew.timer (1)
2-2: LGTM! Timer description updated for clarity.The description now uses "DNS TAPIR" with proper spacing, which aligns with the branding used elsewhere in the project.
main.go (1)
8-8: LGTM! Import path updated correctly.The import path has been updated to
dnstapir-cli/cmd, matching the new module name ingo.mod.Makefile (2)
1-1: LGTM! Program name updated.The
PROGvariable has been correctly updated todnstapir-cli, which will be used throughout the Makefile for build targets.
13-13: LGTM! Spec file path updated.The
SPECFILEvariable now points to the renamed spec file atrpm/SPECS/dnstapir-cli.spec.cmd/root.go (3)
32-33: LGTM! Command name and description updated correctly.The Cobra command's
Usefield has been updated todnstapir-cliand the description now references "POP, i.e. the DNS TAPIR Policy Processor", both aligning with the project rename.
49-49: LGTM! Flag description simplified.The standalone flag description now references "POP process" instead of "TAPIR-POP", providing clearer terminology.
55-55: LGTM! TLS flag description updated.The description now uses "POP process" for consistency with other flag descriptions.
rpm/SPECS/dnstapir-cli.spec (4)
1-1: LGTM! Package name updated.The RPM package name has been correctly renamed to
dnstapir-cli.
10-11: LGTM! Source files renamed correctly.The systemd service and timer source files have been updated to use the
dnstapir-renewprefix.
45-47: LGTM! File paths updated consistently.The configuration file and systemd unit files now use the
dnstapirprefix throughout.
51-51: LGTM! System user renamed correctly.The system user has been appropriately renamed from
tapir-renewtodnstapir-renew, consistent with the overall rename. Note that past review comments indicated the corresponding service filerpm/SOURCES/dnstapir-renew.service(not included in this review) may still need updates to reference this new user name and the correct binary path.
fix #46
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deb/DEBIAN/postinst (1)
4-4: Create the daemon group as a system groupWithout
--system,addgroupmay allocate a regular (high-GID) group, which is not ideal for service accounts. Please creatednstapiras a system group instead.-getent group dnstapir || addgroup dnstapir +getent group dnstapir || addgroup --system dnstapir
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(2 hunks)Makefile(4 hunks)deb/DEBIAN/control(1 hunks)deb/DEBIAN/postinst(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- deb/DEBIAN/control
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Target "PROG" should be declared PHONY.
(phonydeclared)
[warning] 13-13: Target "SPECFILE" should be declared PHONY.
(phonydeclared)
[warning] 59-59: Target body for "deb" exceeds allowed length of 5 (7).
(maxbodylength)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Leon Fernandez <zluudg@zluudg.se>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Makefile (1)
59-66: Add newdebtarget to.PHONYdeclaration.The
debtarget does not produce a file nameddeb/, so it should be declared phony to preventmakefrom skipping it if such a directory exists. Additionally, other existing non-file-producing targets (linux,netbsd,test,install,tarball,srpm) are missing from the phony declaration.Apply this diff to declare all phony targets:
-.PHONY: build clean +.PHONY: build clean linux netbsd test install tarball srpm deb
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Makefile(4 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Target "PROG" should be declared PHONY.
(phonydeclared)
[warning] 13-13: Target "SPECFILE" should be declared PHONY.
(phonydeclared)
[warning] 59-59: Target body for "deb" exceeds allowed length of 5 (7).
(maxbodylength)
🔇 Additional comments (1)
Makefile (1)
43-43: ✓ Previous review comment addressed.The
cleantarget now uses@rm -f *.debwith the force flag, making artifact removal idempotent and resolving the failure on fresh trees.
make sure default cfg file path uses new prefix
zluudg
force-pushed
the
leon/rename-to-dnstapir-prefix
branch
from
49bd44d to
b15fefd
Compare
|
fix #48 |
Summary by CodeRabbit
New Features
Chores
Removals