Skip to content

chore: migrate google-storage-control#8650

Merged
JoeWang1127 merged 41 commits into
googleapis:mainfrom
JoeWang1127:chore/migrate-google-storage-control
Jun 18, 2026
Merged

chore: migrate google-storage-control#8650
JoeWang1127 merged 41 commits into
googleapis:mainfrom
JoeWang1127:chore/migrate-google-storage-control

Conversation

@JoeWang1127

@JoeWang1127 JoeWang1127 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Migration google-storage-control to librarian generate.

Fixes googleapis/librarian#6359
Fixes googleapis/librarian#5955

@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 enables generation for the Google Storage Control v2 library and updates the npm installation command in librarian.js to use a specific cache directory and suppress audit/funding messages. The reviewer suggests avoiding the hardcoded /tmp/npm-cache path to prevent permission conflicts on shared build agents and ensure cross-platform compatibility, recommending a cache directory within the package root instead.


try {
execSync('npm install', {cwd: packageRoot, stdio: 'inherit'});
execSync('npm install --cache=/tmp/npm-cache --no-audit --no-fund', {cwd: packageRoot, stdio: 'inherit'});

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.

medium

Hardcoding /tmp/npm-cache can cause permission conflicts on shared build agents (e.g., if different builds run under different users) and is not cross-platform. Consider using a directory within the package root to ensure a safe, isolated, and cross-platform cache location.

Suggested change
execSync('npm install --cache=/tmp/npm-cache --no-audit --no-fund', {cwd: packageRoot, stdio: 'inherit'});
execSync('npm install --cache=' + path.join(packageRoot, '.npm-cache') + ' --no-audit --no-fund', {cwd: packageRoot, stdio: 'inherit'});

@JoeWang1127 JoeWang1127 force-pushed the chore/migrate-google-storage-control branch from c2c2a06 to 3c27196 Compare June 15, 2026 19:45
@JoeWang1127 JoeWang1127 marked this pull request as ready for review June 16, 2026 14:44
@JoeWang1127 JoeWang1127 requested a review from a team as a code owner June 16, 2026 14:44

@danieljbruce danieljbruce 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.

Just a few nits, but nothing seems particularly risky about this PR to me so approving to unblock librarian migration.

private _universeDomain: string;
private _servicePath: string;
private _log = logging.log('storage');
private _log = logging.log('storage-control');

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.

nit: Do we need to change this log? What improvement does it offer?

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.

Looks like the previous name is not correct since this is storage control client.

Comment thread packages/google-storage-control/LICENSE
@JoeWang1127 JoeWang1127 merged commit 16db10c into googleapis:main Jun 18, 2026
32 checks passed
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.

nodejs: google-storage-control fails to generate in Docker nodejs: Investigate Offline NPM Install Timeout in google-storage-control

2 participants