fix: skip automated PR when no doc content changes#10
fix: skip automated PR when no doc content changes#10sonupreetam wants to merge 7 commits intocomplytime:mainfrom
Conversation
Signed-off-by: sonupreetam <spreetam@redhat.com>
Adds spec.md and tasks.md for the 014-skip-lockfile-only-pr feature using the .specify/templates scaffolding format, documenting the two user stories, acceptance scenarios, requirements, and completed tasks. Made-with: Cursor
Initialises OpenSpec (v1.2.0) with Cursor tooling and creates the full spec-driven artifact set for the 014-skip-lockfile-only-pr feature: proposal → specs (pr-noise-reduction, content-change-detection) → design → tasks. Removes the earlier manually-scaffolded specs/014-*/ files in favour of the canonical openspec/changes/ location. Signed-off-by: sonupreetam <spreetam@redhat.com>
ce213b2 to
5367abf
Compare
- Gitignore .cursor/commands/ and .cursor/skills/ — these are installed by `openspec init --tools cursor` and are tool-managed infrastructure, analogous to .specify/scripts/ and .specify/templates/. Run `openspec init --tools cursor` to restore them locally. - Add specs/014-skip-lockfile-only-pr/spec.md and tasks.md in SpecKit format matching specs/006-go-sync-tool/ convention so the permanent feature spec lives alongside other project specs. Made-with: Cursor
Signed-off-by: sonupreetam <spreetam@redhat.com>
c8190c4 to
ae89f9c
Compare
…ile-only-pr/ The org standard (org-infra PR #185) aligns on SpecKit (specs/NNN-name/) for canonical spec artifacts. openspec/changes/ is tool-managed working state, analogous to .specify/scripts/ — useful locally but not for long-lived project documentation. Gitignore openspec/changes/ and keep only specs/014-skip-lockfile-only-pr/ as the single canonical spec. openspec/config.yaml remains tracked as the project-level tool config. Signed-off-by: sonupreetam <spreetam@redhat.com>
ae89f9c to
8de5d03
Compare
| add-paths: .content-lock.json | ||
| add-paths: | | ||
| .content-lock.json | ||
| content/ |
There was a problem hiding this comment.
MEDIUM: content/ in add-paths will be a no-op: the main branch .gitignore already contains content/docs/projects/*/, and peter-evans/create-pull-request respects .gitignore by default — so git add content/ won't stage any synced doc files.
If the intent (IMP-005) is to show doc-file diffs in the automated PR, the .gitignore rule for content/docs/projects/*/ would also need to be relaxed. Otherwise, consider removing content/ from add-paths to avoid confusion.
| id: title | ||
| if: steps.check.outputs.has_changes == 'true' | ||
| run: | | ||
| IFS=',' read -ra repos <<< "${{ steps.check.outputs.changed_repos }}" |
There was a problem hiding this comment.
LOW: ${{ steps.check.outputs.changed_repos }} is interpolated directly into the shell script before bash executes. While GitHub repo names are restricted to [a-zA-Z0-9._-] making injection impractical here, the safer pattern is to pass the value via an environment variable:
env:
CHANGED_REPOS: ${{ steps.check.outputs.changed_repos }}
run: |
IFS=',' read -ra repos <<< "$CHANGED_REPOS"- Pass changed_repos via env var instead of direct ${{ }} interpolation
in shell script (security hardening, LOW)
- Remove no-op content/ from add-paths — gitignored build artifacts are
never staged by peter-evans/create-pull-request (MEDIUM)
- Mark IMP-005 as superseded in spec: reviewer visibility into changed
doc files is provided by the writeFileManifest output in the PR body,
not by committing gitignored build artifacts
Signed-off-by: sonupreetam <spreetam@redhat.com>
Made-with: Cursor
- Pass changed_repos via env var instead of direct ${{ }} interpolation
in shell script (security hardening, LOW)
- Remove no-op content/ from add-paths — gitignored build artifacts are
never staged by peter-evans/create-pull-request (MEDIUM)
- Mark IMP-005 as superseded in spec: reviewer visibility into changed
doc files is provided by the writeFileManifest output in the PR body,
not by committing gitignored build artifacts
Signed-off-by: sonupreetam <spreetam@redhat.com>
Signed-off-by: sonupreetam <spreetam@redhat.com>
- Pass changed_repos via env :var instead of direct ${{ }} interpolation
in shell script (security hardening, LOW)
- Remove no-op content/ from add-paths — gitignored build artifacts are
never staged by peter-evans/create-pull-request (MEDIUM)
- Mark IMP-005 as superseded in spec: reviewer visibility into changed
doc files is provided by the writeFileManifest output in the PR body,
not by committing gitignored build artifacts
Signed-off-by: sonupreetam <spreetam@redhat.com>
Signed-off-by: sonupreetam <spreetam@redhat.com>
ca5adcb to
b54dfcc
Compare
- Pass changed_repos via env :var instead of direct ${{ }} interpolation
in shell script (security hardening, LOW)
- Remove no-op content/ from add-paths — gitignored build artifacts are
never staged by peter-evans/create-pull-request (MEDIUM)
- Mark IMP-005 as superseded in spec: reviewer visibility into changed
doc files is provided by the writeFileManifest output in the PR body,
not by committing gitignored build artifacts
Signed-off-by: sonupreetam <spreetam@redhat.com>
Signed-off-by: sonupreetam <spreetam@redhat.com>
Made-with: Cursor
b54dfcc to
27638ce
Compare
- Pass changed_repos via env :var instead of direct ${{ }} interpolation
in shell script (security hardening, LOW)
- Remove no-op content/ from add-paths — gitignored build artifacts are
never staged by peter-evans/create-pull-request (MEDIUM)
- Mark IMP-005 as superseded in spec: reviewer visibility into changed
doc files is provided by the writeFileManifest output in the PR body,
not by committing gitignored build artifacts
Signed-off-by: sonupreetam <spreetam@redhat.com>
|
@marcusburghardt Thank you for you feedbacks. I have removed the content/, it no-op you are right. And took care of the other through pass the value via an environment variable. |
Summary
Addresses review feedback from @marcusburghardt on PR #8: the weekly content-sync workflow was opening automated PRs even when no documentation files changed — only the lockfile SHA differed. This made the content approval gate noisy and easy to rubber-stamp.
--update-lock(writes new SHAs to.content-lock.json) then--write(syncs doc content with the new lock, captureshas_changes)if: steps.check.outputs.has_changes == 'true'— PR is skipped when all docs are unchangedsyncResult.hasChanges()to include file-level doc changes (changedRepoFiles) in addition to repo-level project-page changes — previously a repo whose README was unchanged but whose doc sub-pages had new content would emithas_changes=falseOpenSpec
openspec/changes/skip-lockfile-only-pr/— full spec-driven artifact set:proposal.md— why and what changesspecs/pr-noise-reduction/spec.md— workflow gate requirementsspecs/content-change-detection/spec.md—hasChanges()contractdesign.md— key decisions and trade-offstasks.md— 6/6 completeAlso initialises OpenSpec v1.2.0 with Cursor tooling (
.cursor/commands/,.cursor/skills/).Files changed
.github/workflows/sync-content-check.ymlif:guard on PR stepcmd/sync-content/sync.gohasChanges()includeschangedRepoFilescmd/sync-content/sync_test.goTestHasChangestableTest plan
go test -race ./cmd/sync-content/... -run TestHasChanges— all 7 subtests passgo test -race ./cmd/sync-content/...— full suite passesMade with Cursor