Skip to content

Fix minimumReleaseAge and minimumReleaseAgeExclude for PNPM by moving…#5859

Open
aaronmaxlevy wants to merge 1 commit into
microsoft:mainfrom
aaronmaxlevy:aaron_fix_pnpm_minimum_release_age
Open

Fix minimumReleaseAge and minimumReleaseAgeExclude for PNPM by moving…#5859
aaronmaxlevy wants to merge 1 commit into
microsoft:mainfrom
aaronmaxlevy:aaron_fix_pnpm_minimum_release_age

Conversation

@aaronmaxlevy

Copy link
Copy Markdown

… to pnpm-workspace.yaml

Summary

Fixes #5752

Details

This fixes support for PNPM minimumReleaseAge and minimumReleaseAgeExclude by writing them to pnpm-workspace.yaml instead. There is an existing PR open for this #5798 , however that PR moves it to .npmrc instead, whereas according to @iclanton in #5798 (comment) it is preferable to put it in pnpm-workspace.yaml instead (which I agree with). It has been two weeks since that comment was left on #5798 and it hasn't been addressed there, so I figured I'd file my own PR, given that there are real security implications to this setting not working currently. Apologies if this is undesired.

How it was tested

Ran the Rush test suite and validated outputs.

Impacted documentation

N/A

@aaronmaxlevy

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Reve AI"

{
"changes": [
{
"comment": "Fix minimumReleaseAge and minimumReleaseAgeExclude in pnpm-config.json being silently ignored because they were written to package.json instead of pnpm-workspace.yaml",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"comment": "Fix minimumReleaseAge and minimumReleaseAgeExclude in pnpm-config.json being silently ignored because they were written to package.json instead of pnpm-workspace.yaml",
"comment": "Fix `minimumReleaseAge` and `minimumReleaseAgeExclude` in `pnpm-config.json` being silently ignored because they were written to package.json instead of `pnpm-workspace.yaml`",

workspaceYaml.allowBuilds = this._allowBuilds;
}

if (this._minimumReleaseAge !== undefined) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens if this is set to undefined? Is the field omitted from the output YAML in that case? If it is, we don't need this check.

Same question about minimumReleaseAgeExclude. Can that just be set to an empty array? Would that cause any issues?

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

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

[rush] minimumReleaseAge in pnpm-config.json is written to package.json but pnpm only reads it from .npmrc/pnpm-workspace.yaml

2 participants