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 safe schema change for descriptions changing #4275

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Oct 31, 2024

This adds a new safe schema-change when a description changes of any schema-member. Do note that there are some drive-by fixes in here as I noticed a bug in safe-argument changes where we'd just always report the changes rather than actually see whether something changed 😅

Resolves #4245

@JoviDeCroock JoviDeCroock requested a review from a team as a code owner October 31, 2024 18:48
Copy link

netlify bot commented Oct 31, 2024

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit 3cf42a6
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/6723d0e379b78300087859a9
😎 Deploy Preview https://deploy-preview-4275--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

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

@JoviDeCroock JoviDeCroock added the PR: feature 🚀 requires increase of "minor" version number label Nov 1, 2024
@yaacovCR
Copy link
Contributor

yaacovCR commented Nov 1, 2024

as I noticed a bug in safe-argument changes where we'd just always report the changes rather than actually see whether something changed

I broke this out into a separate commit on this fix out into a separate commit on this new branch: https://github.com/graphql/graphql-js/tree/break-out-fix

Instead of doing stacked PRs, we can just have multiple commits within a PR and use the "rebase" merge strategy instead of the "squash" strategy. This makes the git history easier to follow in the long run but doesn't lead to (as much) rebasing madness.

Copy link
Contributor

@yaacovCR yaacovCR left a comment

Choose a reason for hiding this comment

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

Left a comment with something to consider, and addressed separately the drive-by changes. I guess we should try our best to minimize those, I know I've been tempted to sneak them in when possible!

@JoviDeCroock JoviDeCroock merged commit 51e53d7 into main Nov 1, 2024
36 checks passed
@JoviDeCroock JoviDeCroock deleted the add-description-changes branch November 5, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feature 🚀 requires increase of "minor" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants