graphql-js
graphql-js copied to clipboard
Backport 16.x.x into main
We could backport https://github.com/graphql/graphql-js/pull/3730 to v17 as it's reverted in 16.x.x CC @n1ru4l
This gets us to a linear history - in the future as we merge things into 16.x.x let's ensure we leverage merge commits
Deploy Preview for compassionate-pike-271cb3 ready!
| Name | Link |
|---|---|
| Latest commit | 58aca55dc4630907ed6182c92e355c6c183028ac |
| Latest deploy log | https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/66bef346d6a68d000831f102 |
| Deploy Preview | https://deploy-preview-4165--compassionate-pike-271cb3.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Hi @JoviDeCroock, I'm @github-actions bot happy to help you with this PR 👋
Supported commands
Please post this commands in separate comments and only one per comment:
@github-actions run-benchmark- Run benchmark comparing base and merge commits for this PR@github-actions publish-pr-on-npm- Build package from this PR and publish it on NPM
It would be wise to look over https://github.com/graphql/graphql-js/blob/16.x.x/resources/gen-changelog.js and see if it supports merge commits or if support can be added easily. If not, it may make sense to move to something like changesets before this merge? Whatever we do, it should be at least as good as what we have already.
@benjie you are right
Error: Commit 1f8ba95c662118452bd969c6b26ba4e9050c55da has no associated PR: 16.5.0
It looks like the changes center around the following three areas:
- Readme changes around graphql conf
- Enum changes
- instanceof changes and documentation for v16
Maybe we can:
- submit three separate pull requests for these three issues into v17
- run the change log script / release
- do the merge commit
in terms of going forward:
I think a solution is to just release v17 and then go back to having main be stable again ie continue to represent v17 for time being and then any v18 be a separate feature branch
Are we ready to release v17? I think so! All the changes around incremental delivery are only around refactoring with the actual format stable for quite some time.
Two additional points:
This approach would mean that the merge commit itself has no substantial changes which I think would make the history more readily understandable. There would be a single merge commit without any actual changes only for the purpose of creating a non conflicting diff between v16.x.x branch and v17 / main — I am not 100% certain of the importance of creating this non-conflicting diff, but…
In terms of readiness to release v17 — I mean, incremental delivery is ready to release under the experimental tags not that it is ready for actual release.
Are we ready to release v17? I think so! All the changes around incremental delivery are only around refactoring with the actual format stable for quite some time.
I think that is a working group discussion and shouldn't be done lightly, I know there are more things that needed attention from previous WG meetings. In particular the dual package hazard as well as removing instanceof.
This approach would mean that the merge commit itself has no substantial changes which I think would make the history more readily understandable. There would be a single merge commit without any actual changes only for the purpose of creating a non conflicting diff between v16.x.x branch and v17 / main — I am not 100% certain of the importance of creating this non-conflicting diff, but…
It makes the git blame accurate with backrefs to relevant PR's and explanations.
If not, it may make sense to move to something like changesets before this merge? Whatever we do, it should be at least as good as what we have already.
I think either we move to changesets or for this one release we do a manual changelog, I don't mind the latter and moving to changesets after the fact. My main concern is leaving this branch open for too long CC @benjie
Alternatively we add --first-parent to the revList retrieval.
It makes the
git blameaccurate with backrefs to relevant PR's and explanations.
We could have that by frontporting the enum changes, instanceof changes (and the banner) to main without the merge commit? Meaning git blame would identify those lines. It was my understanding that merge commits make git blame more difficult, although I am less familiar with that workflow.
I didn’t mean to be cavalier btw about releasing v17 — apologies!. Just excited in that I believe the incremental delivery experiment should no longer be considered to be blocking.
We could have that by frontporting the enum changes, instanceof changes (and the banner) to main without the merge commit? Meaning git blame would identify those lines. It was my understanding that merge commits make git blame more difficult, although I am less familiar with that workflow.
I understand but it would also take away the contributions from the folks who committed time to improving v16, from that point of view I personally prefer this approach over finding a way around our changelog currently lacking.
I didn’t mean to be cavalier btw about releasing v17 — apologies!. Just excited in that I believe the incremental delivery experiment should no longer be considered to be blocking.
No need to apologize at all, if I came across in any blame way I do apologize. I am very excited about incremental delivery but a new major is always a big effort for our consumers and I personally always fear leaving people behind (if we look at v14/v15, there's still a lot of folks left behind). My reasoning is more in that area, when we cut a major I would love for everyone to chime in and agree that we have everything we want in this major.
@JoviDeCroock If you have time to take a look at #4171 and the discussion I included there, I am trying to show that I think that the very valid concerns around both linear history and author credit can both be addressed whilst still preserving the linear history on main.
Should this be closed now?