Skip to content

update examples with latest request-converter #5865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miguelgrinberg wants to merge 1 commit into
base: main
Choose a base branch
from
+862 −25
Open

update examples with latest request-converter #5865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ overlay-docs: ## Apply overlays to OpenAPI documents
rm output/openapi/elasticsearch-openapi-docs.tmp*.json

generate-language-examples:
@npm update --prefix docs/examples @elastic/request-converter
Copy link
Member

@pquentin pquentin Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should we pin the version in package.json/package-lock.json instead of always updating it?

Copy link
Contributor Author

@miguelgrinberg miguelgrinberg Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The version is set to the latest 9.x.x in docs/example/package.json, so the npm update just makes sure the latest release is pulled. Not sure I understand your suggestion. Are you saying we should edit package.json to have an exact version specified each time this command runs?

Copy link
Member

@pquentin pquentin Dec 19, 2025

Choose a reason for hiding this comment

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

Disclaimer: I'm not super familiar with npm.

The version is set to the latest 9.x.x in docs/example/package.json

It's set to the latest 9.1.x (which I think is what ^9.1.2 does). You probably wanted something like ^9.x instead.

so the npm update just makes sure the latest release is pulled.

It does indeed update to the latest 9.x.x (9.3.0 right now), even though the package.json file forbids it.

Are you saying we should edit package.json to have an exact version specified each time this command runs?

No, but I'm saying we should record the version we're using in the package-lock.json file, like we do for all other dependencies. This would:

  • allow us to use npm clean-install for reproducible builds and offline installs
  • properly record the actual version we're using
  • avoid surprises when trying to figure out the version that was used in CI
  • give us a single mechanism for dependencies versions

Copy link
Contributor Author

@miguelgrinberg miguelgrinberg Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The caret means "compatible with". When you say ^9.1.2 you are saying >= 9.1.2, < 10.0.0. So package.json is correct for 9.3.0 and all future updates until we go to v10. semver docs

but I'm saying we should record the version we're using in the package-lock.json file

I agree in principle, but we do not run this command, the docs team use it. This isn't part of any automated workflows, it is pretty much one of the docs team members running this locally on their laptops to generate a fresh openapi file to upload to the docs site. If we want to enforce package-lock.json to be up to date then it will have to be them that commit it each time they use this command to generate an openapi update.

@node docs/examples/generate-language-examples.js
@npm run format:fix-examples --prefix compiler

Expand Down
Loading
Loading