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

fix: skip packages with version 0.0.0 #89

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Sep 28, 2023

When adding a new package to a monorepo, this script will fail unless the new package is included in the next release. It sees the initial version of 0.0.0 as "new" because it doesn't match the version on npm (which does not exist).

The script has been updated to skip packages with the version 0.0.0 to better accommodate this scenario.

The test has been updated to include an example demonstrating this behavior

To manually test this:

  • Navigate to the core monorepo
  • Checkout the commit for v79
  • Run the get-release-packages.sh script
  • See that the polling controller is not included in the final printed output (ignoring the GITHUB_OUTPUT unbound variable error)

@Gudahtt Gudahtt requested a review from a team as a code owner September 28, 2023 12:57
@Gudahtt

This comment was marked as resolved.

@Gudahtt Gudahtt marked this pull request as draft September 28, 2023 13:52
@Gudahtt Gudahtt force-pushed the skip-packages-with-version-zero branch 4 times, most recently from 42bc835 to fd02a27 Compare September 28, 2023 15:23
When adding a new package to a monorepo, this script will fail unless
the new package is included in the next release. It sees the initial
version of `0.0.0` as "new" because it doesn't match the version on npm
(which does not exist).

The script has been updated to skip packages with the version `0.0.0`
to better accommodate this scenario.
@Gudahtt Gudahtt force-pushed the skip-packages-with-version-zero branch from fd02a27 to a15ee72 Compare September 28, 2023 15:28
@Gudahtt Gudahtt marked this pull request as ready for review September 28, 2023 15:40
@@ -39,7 +39,7 @@ jobs:
while read -r location name; do
if [[ "$name" != "root" ]]; then
PRIVATE=$(jq --raw-output '.private' "$location/package.json")
if [[ "$PRIVATE" != true && "${#PUBLIC_PACKAGES[@]}" -ne 3 ]]; then
if [[ "$PRIVATE" != true && "${#PUBLIC_PACKAGES[@]}" -ne 4 ]]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

This part of the test selects a few packages to modify in the next step. We used to only need 3, but I've upped it to 4 because I've added a new test case (the 0.0.0 case)

@jiexi
Copy link

jiexi commented Sep 28, 2023

Verified. I get the following output. No polling controller

+ RELEASE_PACKAGES='{"packages":{"@metamask/accounts-controller":{"name":"@metamask/accounts-controller","path":"packages/accounts-controller","version":"2.0.1"},"@metamask/address-book-controller":{"name":"@metamask/address-book-controller","path":"packages/address-book-controller","version":"3.1.3"},"@metamask/announcement-controller":{"name":"@metamask/announcement-controller","path":"packages/announcement-controller","version":"4.0.2"},"@metamask/approval-controller":{"name":"@metamask/approval-controller","path":"packages/approval-controller","version":"3.5.2"},"@metamask/assets-controllers":{"name":"@metamask/assets-controllers","path":"packages/assets-controllers","version":"14.0.0"},"@metamask/base-controller":{"name":"@metamask/base-controller","path":"packages/base-controller","version":"3.2.2"},"@metamask/composable-controller":{"name":"@metamask/composable-controller","path":"packages/composable-controller","version":"3.0.2"},"@metamask/controller-utils":{"name":"@metamask/controller-utils","path":"packages/controller-utils","version":"5.0.1"},"@metamask/ens-controller":{"name":"@metamask/ens-controller","path":"packages/ens-controller","version":"5.0.1"},"@metamask/gas-fee-controller":{"name":"@metamask/gas-fee-controller","path":"packages/gas-fee-controller","version":"7.0.1"},"@metamask/keyring-controller":{"name":"@metamask/keyring-controller","path":"packages/keyring-controller","version":"8.0.1"},"@metamask/logging-controller":{"name":"@metamask/logging-controller","path":"packages/logging-controller","version":"1.0.3"},"@metamask/message-manager":{"name":"@metamask/message-manager","path":"packages/message-manager","version":"7.3.4"},"@metamask/name-controller":{"name":"@metamask/name-controller","path":"packages/name-controller","version":"2.0.0"},"@metamask/network-controller":{"name":"@metamask/network-controller","path":"packages/network-controller","version":"13.0.1"},"@metamask/notification-controller":{"name":"@metamask/notification-controller","path":"packages/notification-controller","version":"3.1.2"},"@metamask/permission-controller":{"name":"@metamask/permission-controller","path":"packages/permission-controller","version":"4.1.2"},"@metamask/phishing-controller":{"name":"@metamask/phishing-controller","path":"packages/phishing-controller","version":"7.0.0"},"@metamask/preferences-controller":{"name":"@metamask/preferences-controller","path":"packages/preferences-controller","version":"4.4.2"},"@metamask/rate-limit-controller":{"name":"@metamask/rate-limit-controller","path":"packages/rate-limit-controller","version":"3.0.2"},"@metamask/selected-network-controller":{"name":"@metamask/selected-network-controller","path":"packages/selected-network-controller","version":"2.0.1"},"@metamask/signature-controller":{"name":"@metamask/signature-controller","path":"packages/signature-controller","version":"6.1.1"},"@metamask/transaction-controller":{"name":"@metamask/transaction-controller","path":"packages/transaction-controller","version":"13.0.0"},}}'
+ echo 'RELEASE_PACKAGES={"packages":{"@metamask/accounts-controller":{"name":"@metamask/accounts-controller","path":"packages/accounts-controller","version":"2.0.1"},"@metamask/address-book-controller":{"name":"@metamask/address-book-controller","path":"packages/address-book-controller","version":"3.1.3"},"@metamask/announcement-controller":{"name":"@metamask/announcement-controller","path":"packages/announcement-controller","version":"4.0.2"},"@metamask/approval-controller":{"name":"@metamask/approval-controller","path":"packages/approval-controller","version":"3.5.2"},"@metamask/assets-controllers":{"name":"@metamask/assets-controllers","path":"packages/assets-controllers","version":"14.0.0"},"@metamask/base-controller":{"name":"@metamask/base-controller","path":"packages/base-controller","version":"3.2.2"},"@metamask/composable-controller":{"name":"@metamask/composable-controller","path":"packages/composable-controller","version":"3.0.2"},"@metamask/controller-utils":{"name":"@metamask/controller-utils","path":"packages/controller-utils","version":"5.0.1"},"@metamask/ens-controller":{"name":"@metamask/ens-controller","path":"packages/ens-controller","version":"5.0.1"},"@metamask/gas-fee-controller":{"name":"@metamask/gas-fee-controller","path":"packages/gas-fee-controller","version":"7.0.1"},"@metamask/keyring-controller":{"name":"@metamask/keyring-controller","path":"packages/keyring-controller","version":"8.0.1"},"@metamask/logging-controller":{"name":"@metamask/logging-controller","path":"packages/logging-controller","version":"1.0.3"},"@metamask/message-manager":{"name":"@metamask/message-manager","path":"packages/message-manager","version":"7.3.4"},"@metamask/name-controller":{"name":"@metamask/name-controller","path":"packages/name-controller","version":"2.0.0"},"@metamask/network-controller":{"name":"@metamask/network-controller","path":"packages/network-controller","version":"13.0.1"},"@metamask/notification-controller":{"name":"@metamask/notification-controller","path":"packages/notification-controller","version":"3.1.2"},"@metamask/permission-controller":{"name":"@metamask/permission-controller","path":"packages/permission-controller","version":"4.1.2"},"@metamask/phishing-controller":{"name":"@metamask/phishing-controller","path":"packages/phishing-controller","version":"7.0.0"},"@metamask/preferences-controller":{"name":"@metamask/preferences-controller","path":"packages/preferences-controller","version":"4.4.2"},"@metamask/rate-limit-controller":{"name":"@metamask/rate-limit-controller","path":"packages/rate-limit-controller","version":"3.0.2"},"@metamask/selected-network-controller":{"name":"@metamask/selected-network-controller","path":"packages/selected-network-controller","version":"2.0.1"},"@metamask/signature-controller":{"name":"@metamask/signature-controller","path":"packages/signature-controller","version":"6.1.1"},"@metamask/transaction-controller":{"name":"@metamask/transaction-controller","path":"packages/transaction-controller","version":"13.0.0"},}}'

@Gudahtt Gudahtt merged commit f184ed4 into main Sep 28, 2023
@Gudahtt Gudahtt deleted the skip-packages-with-version-zero branch September 28, 2023 21:45
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.

3 participants