Skip to content

Conversation

@johanix
Copy link
Collaborator

@johanix johanix commented Jan 15, 2026

Summary by CodeRabbit

  • New Features

    • New CLI subcommand now available for use.
  • Chores

    • Updated local development dependency configuration.

✏️ Tip: You can customize this high-level summary in your review settings.

@johanix johanix requested a review from a team as a code owner January 15, 2026 10:34
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

The PR registers a new EdmCmd subcommand in the root command and adds a local module replacement in go.mod to point the tapir dependency to a local directory instead of a remote version.

Changes

Cohort / File(s) Summary
Command Registration
cmd/root.go
Added rootCmd.AddCommand(cmd.EdmCmd) to register the new Edm subcommand
Dependency Configuration
go.mod
Added local module replacement: replace github.com/dnstapir/tapir => ../tapir

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • bump tapir lib #44: Modifies the same go.mod dependency (github.com/dnstapir/tapir) via version updates instead of local replacement
  • Leon/issue/28/nodeman support #30: Also registers new subcommands (EnrollCmd and RenewCmd) in cmd/root.go following the same pattern

Suggested reviewers

  • eest

Poem

🐰 A new command hops into place,
EdmCmd joins the CLI race!
With local tapir paths aligned,
Dependencies restructured and refined. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a new 'dnstapir-cli edm stats' command via EdmCmd registration and go.mod replacement.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In `@go.mod`:
- Around line 5-6: The go.mod currently has a local replace directive "replace
github.com/dnstapir/tapir => ../tapir" which breaks CI; remove that replace
directive from go.mod before merging and either update the require for
github.com/dnstapir/tapir to the published version you need or keep the PR as a
draft until the tapir changes are published so CI can resolve the dependency.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c1c334 and aa24686.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • cmd/root.go
  • go.mod
🧰 Additional context used
🪛 GitHub Actions: Build
cmd/root.go

[error] 16-16: Build failed: replacement directory '../tapir' does not exist for github.com/dnstapir/[email protected] during go build.

🔇 Additional comments (1)
cmd/root.go (1)

57-59: EdmCmd registration follows the established pattern.

The command registration at line 58 is consistent with how PopCmd, DawgCmd, ApiCmd, FilterlistsCmd, EnrollCmd, and RenewCmd are added. The syntax is identical and the import source matches other commands. The go.mod replacement for github.com/dnstapir/tapir is properly configured pointing to the local ../tapir directory.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +5 to +6
replace github.com/dnstapir/tapir => ../tapir

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Remove the local replace directive before merging — this breaks CI.

The replace directive points to a local ../tapir directory that doesn't exist in CI, causing the build to fail. Local replacements are useful during development but should not be committed to main.

Options:

  1. Remove the replace directive and publish the required tapir changes first, then update the version in require.
  2. If the tapir changes aren't ready for release, consider keeping this PR as a draft until they are.
🤖 Prompt for AI Agents
In `@go.mod` around lines 5 - 6, The go.mod currently has a local replace
directive "replace github.com/dnstapir/tapir => ../tapir" which breaks CI;
remove that replace directive from go.mod before merging and either update the
require for github.com/dnstapir/tapir to the published version you need or keep
the PR as a draft until the tapir changes are published so CI can resolve the
dependency.

@oej
Copy link
Member

oej commented Jan 15, 2026

Don't forget to update techdocs

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants