Skip to content

Add 'Assert' consistency to algorithm format check #1168

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

Merged
merged 4 commits into from
Jun 5, 2025
Merged

Add 'Assert' consistency to algorithm format check #1168

merged 4 commits into from
Jun 5, 2025

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