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

Add back "fix: skip PR creation if no prerelease changesets exist"" #11941

Merged

Conversation

jerelmiller
Copy link
Member

Reverts #11939

Adds back the logic there before. I believe the check for no changesets is preventing any new prerelease PRs from getting created if we haven't yet done a prerelease. We'll need to figure out how best to work around this while ensuring we aren't opening prerelease PRs for changes merged to main (which I believe is what this change fixed).

Copy link

changeset-bot bot commented Jul 9, 2024

⚠️ No Changeset found

Latest commit: 3a027ba

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Jul 9, 2024

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 38.68 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 47.48 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 45.05 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 34.22 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 32.08 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.26 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.24 KB (0%)
import { useQuery } from "dist/react/index.js" 5.32 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.39 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 5.6 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.67 KB (0%)
import { useMutation } from "dist/react/index.js" 3.59 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.81 KB (0%)
import { useSubscription } from "dist/react/index.js" 3.23 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.43 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 5.47 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.12 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 4.98 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.63 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" 5.04 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.69 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3.35 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 3.3 KB (0%)
import { useFragment } from "dist/react/index.js" 2.32 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.27 KB (0%)

@jerelmiller jerelmiller merged commit 165c4e4 into main Jul 9, 2024
42 checks passed
@jerelmiller jerelmiller deleted the revert-11939-revert-11729-update-prerelease-workflow branch July 9, 2024 21:03
@alessbell
Copy link
Member

I'm still noodling on the best way to prevent the issue this was attempting to fix, but I'm convinced this is not the way :D So I think we can close this PR for now, or re-revert it :D

The scenario that happened in March:

  • I was cutting a 3.10 alpha
  • a Version Packages (alpha) PR was opened
  • There was an unreleased changeset on main and its slug got included in the array in pre.json
  • I edited the Version Packages (alpha) branch and removed it from the changesets array. I did this because when there are changesets from main that land in the prerelease branch, those commit descriptions wind up in alpha/RC release notes which pollutes the history
  • After merging the Version Packages PR into the release-3.10 branch, the prerelease job re-ran, but instead of just doing the release, the presence of an unreleased changeset (the one on main) caused the version to increment again, so it went to alpha.1 instead of the intended alpha.0. Had I left its slug in the array this wouldn't have happened: that array is append-only, and it doesn't really matter if unreleased changesets wind up in there and never get removed until we exit prerelease mode. But the change would have erroneously wound up in the prerelease notes...

In practice, we should cut prereleases after we've released any pending patch versions from main because it vastly simplifies things. We could automate this by fetching the list of changesets on main and x-referencing it with the changesets on the prerelease branch, and only doing a changeset version if there are pending changesets other than unreleased-from-main ones. If all that sounds good I can create a ticket :)

@alessbell
Copy link
Member

And we don't need to make any changes right now - with this change back in place, it only blocks the first Version Packages (alpha) when the pre.json changesets array is initially empty, as you noted, so we have time to get a proper fix in.

@jerelmiller
Copy link
Member Author

Oops pulled the trigger too soon 😂

In practice, we should cut prereleases after we've released any pending patch versions from main because it vastly simplifies things.

100% this. I'm ok with leaving this as a practice rather than trying to automate this for now.

There was an unreleased changeset on main and its slug got included in the array in pre.json

Does this happen only if we merge main into the release-x branch or does it also happen if there also happens to be changesets present on main. If the former, perhaps we should avoid merging main into the release-x branch until we've released those changes? I'm a bit naive on much of the internal workings here so forgive me if this is a dumb question.

Regardless, thanks for the context here! That is useful 🙂

@alessbell
Copy link
Member

Does this happen only if we merge main into the release-x branch or does it also happen if there also happens to be changesets present on main. If the former, perhaps we should avoid merging main into the release-x branch until we've released those changes? I'm a bit naive on much of the internal workings here so forgive me if this is a dumb question.

Correct, it only happens if we merge main into release-x, so if we accidentally merge main, we can just wait until after the patch release, merge main again and then do the prerelease. Keeping this as a practice for now sounds good 👍

@jerelmiller
Copy link
Member Author

Ok great! If we ever find it becoming a problem because we keep forgetting, we can always add automation later. Thanks so much!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants