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.

Sign up for GitHub

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

Jump to bottom

Sync .pre-commit-config.yaml from this repo #316

Closed
dhruvkb opened this issue Oct 11, 2022 · 4 comments · Fixed by #330 or #334
Closed

Sync .pre-commit-config.yaml from this repo #316

dhruvkb opened this issue Oct 11, 2022 · 4 comments · Fixed by #330 or #334
Labels
🤖 aspect: dx Concerns developers' experience with the codebase ✨ goal: improvement Improvement to an existing user-facing feature good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed

Comments

@dhruvkb
Copy link
Member

dhruvkb commented Oct 11, 2022

Problem

All our repos now use pre-commit for linting but they are all still using their own .pre-commit-config.yaml files with slightly different hooks and configurations.

Description

Centralising the .pre-commit-config.yaml file (and related files like ESLint and Prettier configs) will help us to ensure a consistent code style across all our repos and make it easier to keep them in sync as styles evolve.

Additional context

We already sync a number of other files (notably GitHub actions) from this repo to all the others so there is a precedent for doing this.

References

You can see the current sync code at https://github.com/WordPress/openverse/blob/7c2e6c3724482413688a96ed955440c57d8f61c3/.github/sync.yml.
@dhruvkb dhruvkb added good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed ✨ goal: improvement Improvement to an existing user-facing feature 🤖 aspect: dx Concerns developers' experience with the codebase labels Oct 11, 2022
@AetherUnbound
Copy link
Collaborator

My only concern with this is that we have a number of non-trivial differences between various pre-commit configs, and some configs which are repo-specific. It would be cool if we could use the same mechanism as the docker-compose.override.yml to define a base pre-commit config that the repo can add onto or adapt 😮

@dhruvkb
Copy link
Member Author

dhruvkb commented Oct 20, 2022

Related to #317.

This was referenced Nov 11, 2022
Merged
Merged
Merged
@dhruvkb
Copy link
Member Author

dhruvkb commented Nov 12, 2022 •
edited
Loading

@AetherUnbound I've opened a PR BetaHuhn/repo-file-sync-action#271 for the sync action to support Jinja-style templates. Although it's not 100% what you suggested here, but it'll open a whole slew of possibilities for us to sync even more content centrally!

@AetherUnbound
Copy link
Collaborator

Wow! That's incredible 🤩

@dhruvkb dhruvkb mentioned this issue Nov 22, 2022
Merged
7 tasks
dhruvkb added a commit that referenced this issue Feb 20, 2023
@dhruvkb
Remove redundant steps and use just recipes in CI (#316)
ebad30c
dhruvkb pushed a commit that referenced this issue Feb 20, 2023
@obulat @krysal
Make search and filter stores namespaced (#316)
0e12e3a 
* Make search and filter store namespaced

When merging a query, it is impossible to unset a parameter value, so query replacement was added

Use map helpers and address code review changes

Apply suggestions from code review

Co-authored-by: Krystle Salazar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🤖 aspect: dx Concerns developers' experience with the codebase ✨ goal: improvement Improvement to an existing user-facing feature good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed
Projects
None yet
Milestone
No milestone
2 participants
@AetherUnbound @dhruvkb