-
Notifications
You must be signed in to change notification settings - Fork 213
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
Sync all pre-commit configs from this repo #334
Conversation
8526fcf
to
2ba6341
Compare
1362839
to
5e4cbee
Compare
7bf50f9
to
62781db
Compare
f83476b
to
f275e73
Compare
f275e73
to
616fed7
Compare
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.
This is so excellent, Dhruv! What an exciting development and a creative solution 🚀
I'm getting the following when I attempt to run $ j install
cd automations/python && pipenv install --dev
Installing dependencies from Pipfile.lock (66ca63)...
🐍 ▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉ 1/1 — 00:00:00
Ignoring appnope: markers 'sys_platform == "darwin"' don't match your environment
To activate this project's virtualenv, run pipenv shell.
Alternatively, run a command inside the virtualenv with pipenv run.
cd automations/js && pnpm install
ERR_PNPM_NO_PKG_MANIFEST No package.json found in /home/aether/git-a8c/openverse/automations/js
error: Recipe `_js-install` failed on line 38 with exit code 1
error: Recipe `install` failed on line 14 with exit code 1 |
@@ -36,3 +36,15 @@ _py-install: | |||
# Install dependencies for JavaScript | |||
_js-install: | |||
cd automations/js && pnpm install |
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.
@AetherUnbound does removing cd automations/js
from this line fix the error you saw?
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.
It does!
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.
I can't reproduce this locally, no matter what I tried.
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.
This looks really good, but I get console errors when trying to render the precommit config. The prettier one works fine:
just render \
.pre-commit-config.frontend.yaml.jinja \
.pre-commit-config.frontend.yaml \
'{ "contains_python_code": false }'
cd automations/js && node src/render-jinja.js .pre-commit-config.frontend.yaml.jinja .pre-commit-config.frontend.yaml { "contains_python_code": false }
undefined:1
{
SyntaxError: Unexpected end of JSON input
at JSON.parse (<anonymous>)
at Object.<anonymous> (/Users/zackkrida/Code/openverse/scripts/automations/js/src/render-jinja.js:22:47)
at Module._compile (node:internal/modules/cjs/loader:1155:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
at Module.load (node:internal/modules/cjs/loader:1033:32)
at Function.Module._load (node:internal/modules/cjs/loader:868:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
at node:internal/main/run_main_module:22:47
error: Recipe `render` failed on line 42 with exit code 1
@zackkrida it was a weird case of Just removing the quotes around the arguments. The updated instructions should work. |
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.
@dhruvkb, indeed, that fixed it. LGTM!
One thing I'd note, here we're using jinja and in the frontend we're about to start using remake for a similar goal. We'll want to consolidate these kinds of things long term. |
Jinja support got baked into the |
Fixes
Fixes #316 by @dhruvkb (this time for real)
Description
This PR changes the pre-commit configuration into a Jinja template, and uses the templating feature newly added to the sync action to compile them into the right pre-commit files for the API, the catalog and the frontend.
Testing instructions
To see the compiled configs, follow these steps:
just install
The
render-jinja.js
script simulates the template rendering code present inBetaHuhn/repo-file-sync-action
.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin