Skip to content

CP-13726 Add dependency-bump MD file for dependency-bump skill - #704

Open
SumoSourabh wants to merge 1 commit into
developfrom
dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282
Open

CP-13726 Add dependency-bump MD file for dependency-bump skill#704
SumoSourabh wants to merge 1 commit into
developfrom
dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282

Conversation

@SumoSourabh

@SumoSourabh SumoSourabh commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Jira: https://perforce.atlassian.net/browse/CP-13726

Problem

virtualization-sdk gets a steady stream of bot-raised dependency-update PRs (Dependabot, Mend) across its 5 Python modules (common, libs, platform, tools, dvp) plus a separate docs module built with mkdocs/pipenv. Each lands as its own PR with no build/test verification and no CVE or breaking-change analysis, so reviewing them one at a time is slow and low-context. The new dev-workflow:dependency-bump Claude Code skill (delphix/dlpx-claude-plugins#411) can consolidate a repo's whole backlog into one risk-assessed PR, but it's generic across repos and needs a per-repo config telling it how to build/verify this repo, which bots to watch, and how to handle the docs module's non-pyproject.toml lock file — that config doesn't exist yet for this repo.

Solution

Add .claude/dependency-bump.md, the per-repo configuration the dependency-bump skill reads before touching anything:

  • Bot authors: app/dependabot, app/mend-for-github-com.
  • Version policy: latest-stable (this repo already tracks latest releases rather than pinning conservatively).
  • Build & verification: two passes, never combined — pass 1 (sh bin/build_project.sh -b) always builds all 5 modules unscoped, since coverage/flake8 are only declared in tools/pyproject.toml's dev extra and a scoped build that excludes tools would leave them uninstalled; pass 2 (sh bin/build_project.sh -ctf) runs coverage-instrumented tests + flake8 scoped via -m <module> to only the modules whose pyproject.toml changed (omitted entirely when all 5 are affected).
  • docs module handled separately, since it's pipenv-based (docs/Pipfile/Pipfile.lock), not pyproject.toml: verification runs mkdocs build --clean --strict instead of the repo's own docs/build.sh, which was explicitly ruled out because it publishes straight to the production docs bucket (aws s3 sync ... --delete) as a side effect — unsafe to run against an unmerged batch.
  • Lock-file handling for docs/Pipfile.lock: direct dependencies get their Pipfile line edited then pipenv lock; transitive dependencies (no line in Pipfile) get only the bare re-resolve, never pipenv update <dep>==<version>, since that command silently adds the target as a new direct pin as a side effect.
  • Additional automations: flake8 as a [sync] gate, plus an end-to-end dvp init / dvp build smoke check verifying artifact.json is produced.
  • JIRA: tracking tickets filed under DLPX project, type Bug.

No skill logic changes — this PR only supplies the config so dependency-bump can operate against this repo.

Testing

  • Exercised this exact config (as an unpushed local commit, before opening this PR) via a scoped dry run of the dependency-bump skill against this repo — analyze + build/verify only, no push/JIRA/PR. The skill correctly parsed all sections here, derived the expected per-module -m <module> build plan from which pyproject.toml files a batch touched, and ran the documented bin/build_project.sh verification. That run caught a real pip resolver conflict on httpretty (main-deps range vs. dev-extras exact pin) and, on retry, correctly isolated a genuine behavior change in the bumped httpretty 1.0.5→1.1.4 (double-recording requests in HTTPretty.latest_requests), confirmed against actual HTTP logs.
  • This is a config-only markdown file with no executable code of its own and no automated test suite applies; the dry run above is the verification that it's syntactically well-formed and produces correct behavior when actually consumed by the skill, not just written for eyeballing.

@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch from 65ae87b to 03faa31 Compare September 7, 2026 06:03
@SumoSourabh
SumoSourabh marked this pull request as ready for review September 7, 2026 06:10
@SumoSourabh
SumoSourabh requested a review from a team as a code owner September 7, 2026 06:10
@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch from 03faa31 to d5871a5 Compare September 7, 2026 18:10
@SumoSourabh
SumoSourabh requested a lite review from Copilot September 7, 2026 18:11

Copilot AI 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.

🟡 Changes recommended

The new config contains a few documentation/command accuracy issues (broken Markdown emphasis and incorrect/unclear build/coverage/lock-file guidance) that should be corrected before relying on it for automated dependency PR consolidation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a repository-specific .claude/dependency-bump.md configuration so the dev-workflow:dependency-bump skill can safely consolidate and verify dependency-update PRs for virtualization-sdk, including special handling for the pipenv-based docs module.

Changes:

  • Define which bot authors to consolidate and the repo’s version policy (latest-stable).
  • Document build/verification commands for affected Python modules via bin/build_project.sh (with -m scoping rules).
  • Add docs/-specific verification and pipenv lock-file refresh guidance, plus JIRA metadata and additional sync gates.
File summaries
File Description
.claude/dependency-bump.md New per-repo dependency-bump skill configuration: bots, version policy, verification workflow (including docs/), lock-file handling, and JIRA fields.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .claude/dependency-bump.md Outdated
Comment thread .claude/dependency-bump.md Outdated
Comment thread .claude/dependency-bump.md Outdated
@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch 2 times, most recently from 02952f1 to 9d4709c Compare September 7, 2026 18:33
@SumoSourabh
SumoSourabh requested a lite review from Copilot September 7, 2026 18:35

Copilot AI 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.

🟢 Approval recommended

The change is a config-only addition with no code execution paths modified, and the only feedback is a minor clarity improvement to prevent command misinterpretation.

Review details

Suppressed comments (1)

.claude/dependency-bump.md:28

  • The follow-up verification step is a bit ambiguous about whether -tc and -f should be run together or as separate invocations; making the exact command explicit avoids accidental runs like -c without tests (which won’t generate per-module .coverage files).
**Once the build and test steps pass, run `bin/build_project.sh` again with `-tc` (test coverage) and `-f` (flake8), scoped with the same `-m <module>` flags as above.** Correct any failures and rerun until it passes.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch 6 times, most recently from b3384c1 to 5121807 Compare September 8, 2026 10:48
@SumoSourabh
SumoSourabh requested a lite review from Copilot September 9, 2026 04:42

Copilot AI 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.

🟡 Changes recommended

The documented -m <module> scoping for the two-pass -b then -ctf flow is internally inconsistent and will fail when tools is not included (missing coverage/flake8 installs).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

.claude/dependency-bump.md:36

  • The examples for scoped runs omit tools from pass 1. With the current build_project.sh behavior, that means coverage/flake8 won’t be installed for non-tools-only batches, and pass 2 (-ctf) will fail. The example commands should include -m tools in pass 1 even when only common/libs/platform/dvp manifests changed.
| Source PRs touch                                                | Resolved commands |
|-----------------------------------------------------------------|---|
| Only `libs/pyproject.toml`                                      | `sh bin/build_project.sh -b -m libs` then `sh bin/build_project.sh -ctf -m libs` |
| `libs` + `common`                                               | `sh bin/build_project.sh -b -m libs -m common` then `sh bin/build_project.sh -ctf -m libs -m common` |
| All 5 modules (typical for pytest/zipp/packaging consolidation) | `sh bin/build_project.sh -b` then `sh bin/build_project.sh -ctf` |
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .claude/dependency-bump.md Outdated
@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch from 5121807 to 035939a Compare September 9, 2026 05:00
@SumoSourabh
SumoSourabh requested a lite review from Copilot September 9, 2026 05:01

Copilot AI 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.

🟡 Changes recommended

The docs verification command currently uses pipenv install --skip-lock, which can bypass Pipfile.lock and fail to validate the bumped lockfile correctly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .claude/dependency-bump.md Outdated
@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch from 035939a to f4b8ff5 Compare September 9, 2026 05:05
@SumoSourabh
SumoSourabh requested a lite review from Copilot September 9, 2026 05:06

Copilot AI 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.

🟡 Changes recommended

The added config contains at least one automation command that is not self-contained/reproducible (build_project.sh -f without ensuring dev deps are installed), which could cause the dependency-bump workflow to fail in fresh environments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread .claude/dependency-bump.md
Comment thread .claude/dependency-bump.md
@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch from f4b8ff5 to 58ea774 Compare September 9, 2026 05:30

Copilot AI 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.

🟡 Changes recommended

The new config contains a couple of inaccurate/broken internal references that could mislead users/automation consuming this file.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread .claude/dependency-bump.md Outdated
Comment thread .claude/dependency-bump.md Outdated
Comment thread .claude/dependency-bump.md Outdated
@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch from 58ea774 to fa7b77c Compare September 9, 2026 05:41
@SumoSourabh
SumoSourabh requested a balanced review from Copilot September 9, 2026 05:43

Copilot AI 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.

🟡 Changes recommended

The smoke test pollutes the checkout, and required appdata_python_samples coverage is missing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread .claude/dependency-bump.md Outdated
Comment thread .claude/dependency-bump.md
@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch 2 times, most recently from d27b797 to 5857003 Compare September 9, 2026 05:54
@SumoSourabh
SumoSourabh requested a balanced review from Copilot September 9, 2026 05:54

Copilot AI 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.

🟢 Approval recommended

The configuration is consistent with the repository’s build tooling and addresses the previously identified issues.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@SumoSourabh
SumoSourabh force-pushed the dlpx/pr/SumoSourabh/a9cd4473-078c-472f-ac7f-0d4f8c624282 branch from cfffff1 to 9a08f05 Compare September 11, 2026 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants