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

Capitalisation: Add camelCase #5777

Merged
merged 14 commits into from
Jun 9, 2024
Merged

Capitalisation: Add camelCase #5777

merged 14 commits into from
Jun 9, 2024

Conversation

WittierDinosaur
Copy link
Contributor

@WittierDinosaur WittierDinosaur commented Apr 16, 2024

Brief summary of the change made

This PR adds camelCase to the extended capiltalisation policy. Added tests to cover its functionality.
Fixes #5768

Are there any other side effects of this change that we should be aware of?

I made it so the complex types are all only available when explicitly set in the config.
All 3 are either destructive to the code, or could allow poorly linted files through. Most people would use upper or lower anyway, so requiring the complex cases to be explicit should be a non-issue.

This will require a minor version bump.

Pull Request checklist

  • Please confirm you have completed any of the necessary steps below.

  • Included test cases to demonstrate any code changes, which may be one or more of the following:

    • .yml rule test cases in test/fixtures/rules/std_rule_cases.
    • .sql/.yml parser test cases in test/fixtures/dialects (note YML files can be auto generated with tox -e generate-fixture-yml).
    • Full autofix test cases in test/fixtures/linter/autofix.
    • Other.
  • Added appropriate documentation for the change.

  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

Copy link
Contributor

github-actions bot commented Apr 17, 2024

Coverage Results ✅

Name    Stmts   Miss  Cover   Missing
-------------------------------------
TOTAL   17798      0   100%

225 files skipped due to complete coverage.

@coveralls
Copy link

coveralls commented Apr 17, 2024

Coverage Status

coverage: 99.985%. remained the same
when pulling eb32509 on feat-camelCase
into 9f1446f on main.

@WittierDinosaur WittierDinosaur changed the title Caplitalisation: Add camelCase Capitalisation: Add camelCase Apr 17, 2024
Copy link
Member

@alanmcruickshank alanmcruickshank left a comment

Choose a reason for hiding this comment

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

Can you elaborate in the docs? I think the logic makes sense but I'm having trouble checking that it's consistent without it clearly documenting what it's supposed to do.

Comment on lines 72 to 75
"definition": (
"The capitalisation policy to enforce, extended with PascalCase "
"and snake_case. "
"The capitalisation policy to enforce, extended with PascalCase, "
"snake_case, and camelCase. "
"This is separate from ``capitalisation_policy`` as it should not be "
Copy link
Member

Choose a reason for hiding this comment

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

I think the precedence rules here aren't very easy to guess. I think from running through the examples below, that I understand the logic - but I think we should document it here.

If nothing else - someone will ask on the slack channel and I'd like to be able to point them to something to explain why the default config is acting the way that it is.

@nstringham
Copy link

Any update on this?

@atishay
Copy link
Contributor

atishay commented Jun 1, 2024

How can we help getting this merged?

Copy link
Member

@alanmcruickshank alanmcruickshank left a comment

Choose a reason for hiding this comment

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

LGTM. I agree there's some strange inference on fixing camel case situations, but I don't have any better ideas!

@alanmcruickshank alanmcruickshank added this pull request to the merge queue Jun 9, 2024
Merged via the queue into main with commit 6951328 Jun 9, 2024
32 checks passed
@alanmcruickshank alanmcruickshank deleted the feat-camelCase branch June 9, 2024 19:36
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.

add camel case to extended_capitalisation_policy
5 participants