-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix capitalization in service fee query (id 4017925) #10
Conversation
@fhenneke In case you have other stylistic suggestions, this might be the PR to apply them. E.g, column names. Should we use underscores and lowercase letters instead of names such as |
Actually pushed a change to use snake case throughout |
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.
LGTM, what's the reason for removing capitalization? I thought capitalizing SQL specific words was the standard syntax.
cowprotocol/accounting/rewards/mainnet/service_fee_query_4017925.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/service_fee_query_4017925.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/service_fee_query_4017925.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/service_fee_query_4017925.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/service_fee_query_4017925.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/service_fee_query_4017925.sql
Outdated
Show resolved
Hide resolved
Yeah that's a valid point. I guess currently we are inconsistent and i decided to propose going with lower case. Whatever we choose, it would be nice to stick to it. Do you have any preference @fhenneke ? |
…25.sql Co-authored-by: bram-vdberg <[email protected]>
…25.sql Co-authored-by: bram-vdberg <[email protected]>
…25.sql Co-authored-by: bram-vdberg <[email protected]>
…25.sql Co-authored-by: bram-vdberg <[email protected]>
…25.sql Co-authored-by: bram-vdberg <[email protected]>
…25.sql Co-authored-by: bram-vdberg <[email protected]>
I do not really have an opinion on capitalization. The most important part to me would be would be that it is consistens (first within a query and then across queries). If we cafe about a specific styling, we can add it to the linting check, e.g. via
|
Ok, i would suggest we keep this as is then (lower case) and try to be consistent throughout. Merging so that we can finally start fixing the query |
Trivial PR that lowercases the service fee query and uses snake case throughout for column names (4017925)