Skip to content

chore: updated makefile - #373

Merged
rebEllieous merged 2 commits into
mainfrom
feature/13-update-stale-common.mk
May 29, 2026
Merged

chore: updated makefile#373
rebEllieous merged 2 commits into
mainfrom
feature/13-update-stale-common.mk

Conversation

@rebEllieous

@rebEllieous rebEllieous commented May 21, 2026

Copy link
Copy Markdown
Contributor

What

see opendefensecloud/dev-kit#15 for upstream PR

Summary by CodeRabbit

  • Chores
    • Improved build configuration management by implementing an explicit target recipe for fetching and validating the common build configuration file.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The Makefile refactors its common.mk download mechanism from an inline command into a dedicated make target. The target downloads common.mk from the OpenDefenseCloud dev-kit repository, records the DEV_KIT_VERSION, and creates a completion marker file.

Changes

Build tooling

Layer / File(s) Summary
Common.mk download target
Makefile
The common.mk make target downloads from GitHub raw, writes the resolved DEV_KIT_VERSION to .common.mk-version, and touches .common.mk-checked to mark completion, replacing the previous inline curl && mv mechanism.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • olzemal

Poem

🐰 A rabbit refactors with glee,
Explicit targets, make files so clean,
Download and check with markers so true,
Build scripts that work like they ought to do! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete, providing only a reference link without filling in required template sections like 'Why', 'Testing', 'Notes for reviewers', or the checklist. Complete the PR description template by adding motivation, testing details, any relevant reviewer notes, and confirming the checklist items.
Title check ❓ Inconclusive The title 'chore: updated makefile' is vague and generic, using the non-descriptive term 'updated' without conveying what specific change was made to the Makefile. Provide a more specific title that describes the actual change, such as 'chore: add common.mk download target to Makefile' or 'chore: refactor common.mk fetching mechanism'.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/13-update-stale-common.mk

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.

@coveralls

coveralls commented May 21, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 26624758378

Coverage increased (+1.4%) to 85.022%

Details

  • Coverage increased (+1.4%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 928
Covered Lines: 789
Line Coverage: 85.02%
Coverage Strength: 1302.76 hits per line

💛 - Coveralls

@rebEllieous
rebEllieous enabled auto-merge May 29, 2026 07:40

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Makefile`:
- Around line 6-7: The curl invocation in the Makefile (the line starting with
"curl --fail -sSL
https://raw.githubusercontent.com/opendefensecloud/dev-kit/$(DEV_KIT_VERSION)/common.mk
-o .common.mk-download") lacks timeout flags; update that command to include a
connection timeout (e.g., --connect-timeout 10) and a total timeout (e.g.,
--max-time 60) so the download cannot hang indefinitely, and optionally add
sensible retry flags (e.g., --retry 3) if desired.
- Around line 5-7: The current Makefile check skips downloading if
.common.mk-download exists but curl -o can leave a partial file on failure,
preventing retries; update the download logic that references
.common.mk-download so failed attempts are not cached: either remove the
existence check and let curl fail (so CI/Make will retry), or implement an
atomic download pattern—curl to a temporary file (e.g., .common.mk-download.tmp)
with --fail and then mv to .common.mk-download only on success, and ensure the
temp file is removed on error—apply this change to the block that runs curl
--fail -sSL
https://raw.githubusercontent.com/opendefensecloud/dev-kit/$(DEV_KIT_VERSION)/common.mk
-o .common.mk-download.
- Around line 4-10: The Makefile target "common.mk" currently has no
prerequisites so changing DEV_KIT_VERSION doesn't re-run the download; update
the target so its recipe is re-executed when DEV_KIT_VERSION changes by making
"common.mk" depend on the stamp file ".common.mk-version" (or vice versa) and
write the current DEV_KIT_VERSION into that stamp, or mark the target phony and
gate the download using the contents of ".common.mk-version"; specifically
adjust the target for common.mk, its recipe that writes ".common.mk-version",
and the stamp logic around ".common.mk-version" so Make treats the target as
out-of-date when DEV_KIT_VERSION changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 04dc3610-4ba6-49a9-81f1-a16d3e3c1e68

📥 Commits

Reviewing files that changed from the base of the PR and between be2febf and 7ab0a68.

📒 Files selected for processing (1)
  • Makefile

Comment thread Makefile
Comment thread Makefile
Comment thread Makefile
@rebEllieous
rebEllieous merged commit 61d9279 into main May 29, 2026
16 checks passed
@rebEllieous
rebEllieous deleted the feature/13-update-stale-common.mk branch May 29, 2026 08:02
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.

4 participants