Skip to content
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.

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

Fixes publish script to require NPM tag, use Wombat registry, and correctly read check flags #18324

Merged
merged 3 commits into from
Jul 23, 2020

Conversation

dgp1130
Copy link
Collaborator

@dgp1130 dgp1130 commented Jul 21, 2020

This is a patch PR for #18219. The dist-tag changes are not present here because that script does not exist and would not be used from this branch.

I also dropped the doc: commit since we don't typically look at docs on the non-master branch.

Pure refactor that just pulls the Wombat registry into a separate string constant to be re-used across scripts.
Flag arguments provided by minimist are always strings. Both `--branchCheck` and `--versionCheck` failed to take this into account and were incorrectly typed as a result. Now boolean flags are parsed into actual boolean types which can be used more intuitively.
@dgp1130 dgp1130 added the target: lts This PR is targeting a version currently in long-term support label Jul 21, 2020
@dgp1130 dgp1130 requested review from kyliau and clydin July 21, 2020 22:28
`--tag` now must be explicitly specified rather than defaulting to `latest`. It is also validated to be one of the expected set of tags. There is a new `--tagCheck false` flag that will skip this in order to handle one-off custom tags if necessary in the future.

Apparently NPM requires that all releases have tags (defaulting to `latest` if not explicitly specified), so there is no way to choose **not** to push a new tag. That means that you can't push `10.0.x` and then push `9.0.x` without either using `v9-lts` or clobbering `latest`. The `--tag` requirement should hopefully enforce this restriction to avoid a potential release footgun.
@dgp1130 dgp1130 added the action: merge The PR is ready for merge by the caretaker label Jul 22, 2020
@dgp1130
Copy link
Collaborator Author

dgp1130 commented Jul 22, 2020

This is stuck on 'Missing required status "ci/circleci: integration"'. Is there something more I need to do or is CI trying to run checks that don't exist on this branch?

@alan-agius4
Copy link
Collaborator

alan-agius4 commented Jul 22, 2020 via email

@alan-agius4 alan-agius4 merged commit 90bac99 into angular:9.1.x Jul 23, 2020
@dgp1130 dgp1130 deleted the publish-v9 branch July 23, 2020 17:51
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: lts This PR is targeting a version currently in long-term support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants