Skip to content

fix: Add npm ci target and dependency on all npm run commands #5683

Open
bennerv wants to merge 1 commit into
Azure:mainfrom
bennerv:update-api-makefile-target
Open

fix: Add npm ci target and dependency on all npm run commands #5683
bennerv wants to merge 1 commit into
Azure:mainfrom
bennerv:update-api-makefile-target

Conversation

@bennerv

@bennerv bennerv commented Jun 16, 2026

Copy link
Copy Markdown
Member

No jira

What

Adds an npm ci dependency on any npm run commands as we need the dependencies installed in order to succeed first.

Why

if dependencies are updated, this will force the user to have the latest dependencies for generation.

Testing

api validation should cover this from a prowjob

Special notes for your reviewer

PR Checklist

  • PR is scoped to a single task (no mixed concerns)
  • Title follows Conventional Commits format
  • Summary explains the "Why" behind the change
  • Linked to relevant ticket/issue
  • Screenshots included (if graph/UI/metrics changes)
  • Self-reviewed the diff
  • CI/CD checks are passing (ignore Tide)
  • Draft PR used for WIP (if applicable)
  • Commit history is clean (rebased/squashed)
  • Tricky code blocks are commented
  • Specific reviewers tagged
  • All comment threads resolved before merge

Copilot AI review requested due to automatic review settings June 16, 2026 21:45
@openshift-ci openshift-ci Bot requested review from mbarnes and stevekuznetsov June 16, 2026 21:45
@openshift-ci

openshift-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bennerv, tsatam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the api build/generation Makefile so that any targets invoking npm run … first ensure Node dependencies are installed via an npm ci step, reducing failures due to missing/old dependencies during API generation workflows.

Changes:

  • Add a new ci Makefile target that runs npm ci.
  • Make swagger, models, testsdk, and fmt depend on ci so dependencies are installed before npm run scripts execute.

Comment thread api/Makefile
@bennerv bennerv force-pushed the update-api-makefile-target branch from 925f88f to 468a786 Compare June 16, 2026 21:49
@bennerv bennerv force-pushed the update-api-makefile-target branch from 468a786 to 6a8987d Compare June 16, 2026 22:07
Copilot AI review requested due to automatic review settings June 16, 2026 22:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

api/Makefile:51

  • This Makefile now ensures deps for npm-run targets, but validate-examples/lint-openapi invoke pinned Node tools via npx (oav/autorest). Without depending on the same install stamp, npx may download tools from the network instead of using package-lock-pinned versions. Consider adding the npm ci stamp prerequisite here too, and switch fmt to the stamp file as well.
fmt: node_modules/.package-lock.json
	npm run format

.PHONY: validate-examples
validate-examples:

Comment thread api/Makefile

.PHONY: swagger
swagger:
swagger: node_modules/.package-lock.json
Comment thread api/Makefile

.PHONY: models
models: $(GOIMPORTS)
models: node_modules/.package-lock.json $(GOIMPORTS)
Comment thread api/Makefile

.PHONY: testsdk
testsdk: $(GOIMPORTS)
testsdk: node_modules/.package-lock.json $(GOIMPORTS)
Comment thread api/Makefile
Comment on lines +77 to +78
node_modules/.package-lock.json: package-lock.json
npm ci No newline at end of file
@openshift-ci

openshift-ci Bot commented Jun 17, 2026

Copy link
Copy Markdown

@bennerv: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-parallel 6a8987d link true /test e2e-parallel

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants