Skip to content

Conversation

benjie
Copy link
Member

@benjie benjie commented Jun 2, 2025

We have a mixture of - Assert: blah and - Assert blah. We should standardize on one pattern; this adds it to the spec format checks. (It also fixes some minor bugs in the script itself.)

Copy link

netlify bot commented Jun 2, 2025

Deploy Preview for graphql-spec-draft ready!

Name Link
🔨 Latest commit 9405021
🔍 Latest deploy log https://app.netlify.com/projects/graphql-spec-draft/deploys/683d3c8a4dae750008c66990
😎 Deploy Preview https://deploy-preview-1168--graphql-spec-draft.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 project configuration.

@benjie benjie added the ✏️ Editorial PR is non-normative or does not influence implementation label Jun 2, 2025
Copy link
Contributor

@martinbonnin martinbonnin left a comment

Choose a reason for hiding this comment

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

Nit: I like it better without the : since we don't have Return: foo but Return foo and more generally I find that the spec are usually closer to natural language but consistency > everything so I'm fine with everything and support enforcing this in the format check 👍

@benjie
Copy link
Member Author

benjie commented Jun 5, 2025

Happy to change to the reverse if the group agrees. I favour Assert: because assertions are kind of optional/documentation steps - they should hold already and should never trigger unless there's a bug - so they could theoretically be omitted from the algorithm. The same cannot be said for a "return" statements 😉

They're more like Note: ... to me than Return ....

@leebyron
Copy link
Collaborator

leebyron commented Jun 5, 2025

love this, thanks for adding the check

@leebyron leebyron merged commit 3b0d8e6 into main Jun 5, 2025
9 checks passed
@leebyron leebyron deleted the assert branch June 5, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✏️ Editorial PR is non-normative or does not influence implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants