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 a SQL style guide #555

Merged
merged 2 commits into from
May 22, 2024
Merged

Add a SQL style guide #555

merged 2 commits into from
May 22, 2024

Conversation

zachmargolis
Copy link
Contributor

Starts a small SQL style guide! Like it says inside, it's not enforced, but I would like to start norming as a team on these!

I found that Gitlab has a style guide which I skimmed for reference.

I would like to find a way to use an automated tool like sqlfluff that they mention, but currently, it would not work for our codebases that have SQL embedded inside ruby files. I've got some ideas for how we can cobble something togethe.

@benjaminchait
Copy link
Contributor

@zachmargolis - LGTM, but a question: Would we want to link to external style guides, as references?

benjaminchait
benjaminchait previously approved these changes May 22, 2024
@zachmargolis
Copy link
Contributor Author

Would we want to link to external style guides, as references?

Yes! I skimmed a few but hadn't found any that seemed like good baselines, do you have any suggestions?

@benjaminchait
Copy link
Contributor

benjaminchait commented May 22, 2024

@zachmargolis - I don't offhand, but I will reach-out to a few former colleagues and friends to see if any of them have suggestions. Maybe we can merge this as-is, and we'll plan to add more as we identify some examples?

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Is there meant to be content included in the changes here? I only see a subcategory metadata added.

@zachmargolis
Copy link
Contributor Author

Is there meant to be content included in the changes here? I only see a subcategory metadata added.

d'oh! updated in c580932

@benjaminchait
Copy link
Contributor

@aduth - Amazing catch!

@zachmargolis - I honestly thought this was "let's start the smallest possible thing and iterate" so I didn't question 😅


@zachmargolis - A few possible "best practices" that I've learned about...

@zachmargolis
Copy link
Contributor Author

Thanks! I'll take a closer look at those and see what we can reference/incorporate!

@zachmargolis zachmargolis merged commit d920f6f into main May 22, 2024
5 checks passed
@zachmargolis zachmargolis deleted the margolis-sql-style-guide branch May 22, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants