Skip to content

feat(nodejs): add a DefaultVersion field to NodeJSPackage#6358

Merged
jskeet merged 1 commit into
googleapis:mainfrom
jskeet:node-default-version
Jun 10, 2026
Merged

feat(nodejs): add a DefaultVersion field to NodeJSPackage#6358
jskeet merged 1 commit into
googleapis:mainfrom
jskeet:node-default-version

Conversation

@jskeet

@jskeet jskeet commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Adds a DefaultVersion field to the Node config, which is then written to .repo-metadata.json, overriding the default of "the first API path variant". The combine-libraries part of the post-processor is also provided with the default version to create the appropriate index.ts.

Like Python, our Node packages have the concept of a "default version", which is the API variant which is imported implicitly.

See #6357 for further work we'll need to consider post-migration.

@jskeet jskeet requested a review from quirogas June 10, 2026 08:18
@jskeet jskeet requested a review from a team as a code owner June 10, 2026 08:18

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a DefaultVersion configuration option for Node.js packages, allowing the default API version to be overridden rather than defaulting to the first API path. It updates the post-processor and repository metadata generation to utilize this version, and adds comprehensive unit tests. The feedback suggests renaming the helper function deriveDefaultVersion to resolveDefaultVersion (along with its call sites and tests) to comply with repository naming standards, and removing an extra blank line in the test file to follow the style guide's spacing rules.

Comment thread internal/librarian/nodejs/generate.go Outdated
Comment thread internal/librarian/nodejs/generate.go Outdated
Comment thread internal/librarian/nodejs/generate.go Outdated
Comment thread internal/librarian/nodejs/generate_test.go
Comment thread internal/librarian/nodejs/generate_test.go Outdated
@jskeet jskeet force-pushed the node-default-version branch 2 times, most recently from 9961659 to ac19e99 Compare June 10, 2026 08:44
@jskeet

jskeet commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a default_version configuration option for Node.js packages, allowing users to override the default API version instead of always defaulting to the first API path. It updates the post-processor and repository metadata generation to use this resolved default version, and adds corresponding unit tests. The review feedback points out a potential runtime panic in resolveDefaultVersion if library.APIs is empty, and suggests adding a guard check along with a test case to handle this scenario gracefully.

Comment thread internal/librarian/nodejs/generate.go
Comment thread internal/librarian/nodejs/generate_test.go
@jskeet jskeet force-pushed the node-default-version branch from ac19e99 to d317bf7 Compare June 10, 2026 08:48
Adds a DefaultVersion field to the Node config, which is then written
to .repo-metadata.json, overriding the default of "the first API path
variant". The combine-libraries part of the post-processor is also
provided with the default version to create the appropriate index.ts.

Like Python, our Node packages have the concept of a "default
version", which is the API variant which is imported implicitly.

See googleapis#6357 for further work we'll need to consider post-migration.
@jskeet jskeet force-pushed the node-default-version branch from d317bf7 to a7f9bb7 Compare June 10, 2026 08:50
jskeet added a commit to jskeet/google-cloud-node that referenced this pull request Jun 10, 2026
This is set even if it would be the default anyway, so that changes in
ordering don't affect generation.

See googleapis/librarian#6358 for the
introduction of the field.
@jskeet

jskeet commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

jskeet added a commit to jskeet/google-cloud-node that referenced this pull request Jun 10, 2026
This is set even if it would be the default anyway, so that changes in
ordering don't affect generation.

See googleapis/librarian#6358 for the
introduction of the field.
jskeet added a commit to jskeet/google-cloud-node that referenced this pull request Jun 10, 2026
This is set even if it would be the default anyway, so that changes in
ordering don't affect generation.

See googleapis/librarian#6358 for the
introduction of the field.
jskeet added a commit to jskeet/google-cloud-node that referenced this pull request Jun 10, 2026
This is set even if it would be the default anyway, so that changes in
ordering don't affect generation. The value is fetched from the
existing .repo-metadata.json.

See googleapis/librarian#6358 for the
introduction of the field.

Additionally, updates the Librarian version to v0.20.0, which is the
Librarian version containing this field.
// resolveDefaultVersion returns the default API version (v1, v1beta etc) for
// a library, using the Node-specific override if present, or the path of the
// first API otherwise. If the library has no override and no APIs, an empty
// string is returned.

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.

Suggested change
// string is returned.
// string is returned.
// TODO(https://github.com/googleapis/librarian/issues/6357): determine whether we should keep or remove this function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolving for now, merging, then will unresolve and create this in another PR

@jskeet jskeet merged commit af3218f into googleapis:main Jun 10, 2026
29 checks passed
@jskeet jskeet deleted the node-default-version branch June 10, 2026 17:38
jskeet added a commit to googleapis/google-cloud-node that referenced this pull request Jun 10, 2026
This is set even if it would be the default anyway, so that changes in
ordering don't affect generation. The value is fetched from the
existing .repo-metadata.json.

See googleapis/librarian#6358 for the
introduction of the field.

Additionally, updates the Librarian version to v0.20.0, which is the
Librarian version containing this field.
jskeet pushed a commit that referenced this pull request Jun 10, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.20.0](v0.19.0...v0.20.0)
(2026-06-10)


### Features

* **nodejs:** add a DefaultVersion field to NodeJSPackage
([#6358](#6358))
([af3218f](af3218f))
* **sidekick/rust:** add bigquery code gen
([#6322](#6322))
([a7846f5](a7846f5))
* **sidekick/swift:** non-string maps
([#6361](#6361))
([2b6d7e4](2b6d7e4))
* **sidekick/swift:** support discovery-based modules
([#6351](#6351))
([09ef5cf](09ef5cf))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
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.

2 participants