-
Notifications
You must be signed in to change notification settings - Fork 107
refractor(splinter): allow non-supabase rules to run always #597
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
Conversation
- Remove empty line after outer attribute in pgls_diagnostics - Replace useless format! with .to_string() in pgls_splinter - Refactor loops to use .enumerate() and slice indexing in xtask_codegen - Add clippy and git commit guidelines to AGENTS.md
juleswritescode
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very cool! i like how you integrated this into the LSP!
|
|
||
| let results = query::load_splinter_results(params.conn).await?; | ||
| // Only run Supabase-specific rules if the required roles exist | ||
| let has_supabase_roles = check_supabase_roles(params.conn).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! curious: did the supabase team confirm that this check is the best way to identify a supabase database?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, but for squawk this is what is required to exist for the rules query to not crash :D
| result | ||
| } | ||
|
|
||
| fn split_queries(content: &str) -> (String, String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as I understand it, rules are per default put into the generic category.
what happens if supabase adds a new rule to the spliter that's supabase-specific?
that might break linting in case a table isn't defined for non-supabase databases, right?
should we throw during build if there's an unknown rule in the splinter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great catch!
manually splits the
splinter.sqlfile into generic and supabase-only rules during download inbuild.rs.based on the existence of the roles, run either one or both.
also improved the url handling - no manual fix needed anymore.