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 support for Lektor extensions, additional tool config rcfiles and common plain-text meta-files lacking extensions #156

Merged
merged 3 commits into from Nov 23, 2020

Conversation

CAM-Gerlach
Copy link
Contributor

@CAM-Gerlach CAM-Gerlach commented Nov 22, 2020

  • Add .lr files from the Lektor static CMS, and .lektorproject which == INI
  • Add several config/rc files without extensions from additional relatively popular linting/etc. tools that have pre-commit hooks (.browserslistrc, .codespellrc, .csslintrc, .gitlint, pylintrc). I took care to only add such files where the un-extensioned config file had only one defined file type, as opposed to cases where it could potentially be in one of several
  • Add a few additional common plain text meta filenames (CHANGELOG, CONTRIBUTING, COPYING.LESSER NEWS) with relatively wide usage

There was one ambiguity here: Should ordering the NAMES dict be case-sensitive (i.e. should capitalized letters have higher precedence than lowercase)? Currently, this wasn't possible to infer from the existing order, as there were no cases I could find where this would have made a difference to ordering, but this PR adds one such instance (pylintrc). I arbitrarily picked non-case-sensitive ordering for now, but I'm happy to change it if desired, just lmk. Thanks!

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

  • rebase
  • fix the test that's failing
  • provide links in the PR to the individual configuration formats

identify/extensions.py Outdated Show resolved Hide resolved
identify/extensions.py Outdated Show resolved Hide resolved
identify/extensions.py Outdated Show resolved Hide resolved
identify/extensions.py Outdated Show resolved Hide resolved
@CAM-Gerlach
Copy link
Contributor Author

CAM-Gerlach commented Nov 22, 2020

Thanks for the quick review!

rebase

Done.

fix the test that's failing

Not sure what you mean, all the tests were green before and are now?

provide links in the PR to the individual configuration formats

Done, also updated for the changes you mentioned.

identify/extensions.py Outdated Show resolved Hide resolved
@asottile
Copy link
Member

fix the test that's failing

Not sure what you mean, all the tests were green before and are now?

I added a test on master -- you fixed it as part of fixing one of your two typos :)

identify/extensions.py Outdated Show resolved Hide resolved
identify/extensions.py Outdated Show resolved Hide resolved
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile asottile merged commit 3f16a56 into pre-commit:master Nov 23, 2020
@CAM-Gerlach CAM-Gerlach deleted the add-more-metafiles branch November 23, 2020 02:46
@CAM-Gerlach
Copy link
Contributor Author

KITTEH 😸

@asottile
Copy link
Member

miao miao

@CAM-Gerlach
Copy link
Contributor Author

CAM-Gerlach commented Nov 23, 2020

Meows in Spanish

Maybe a little out of scope, but why not defer the request until the click event fires for the approve radio button? Seems like it would save a lot of extra requests when doing loading any PR page without approving it, especially when clueless contributors like me keep making typos, heh. Too much latency needing to wait for the request after clicking approve, or just not optimized?

@asottile
Copy link
Member

too much latency, yeah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants