Format yaml#487
Conversation
a51a656 to
1448642
Compare
1448642 to
d52812d
Compare
d52812d to
4ad6175
Compare
| check-pyupgrade: | ||
| pyupgrade --py310-plus $$(find . -name '*.py') | ||
|
|
||
| cleanup-yaml: |
There was a problem hiding this comment.
I don't like the usage of docker in this target.
At least it leaves an unused image when executing locally.
There was a problem hiding this comment.
The reason is for prettiers dependence on node, which is otherwise not used in meta.
We could add a target that just assumes the host machine already has node setup and can execute prettier without issue
| paths = src/ tests/ | ||
|
|
||
| all: pyupgrade black autoflake isort flake8 mypy sqlfluff | ||
| lint: pyupgrade black autoflake isort flake8 mypy sqlfluff |
There was a problem hiding this comment.
Does this renaming have effect on CI somewhere?
If not, we should add a check with linting.
There was a problem hiding this comment.
I have not seen "all" being referenced anywhere. It isn't called in workflows, but the composite make targets are. It should probably remain that way, so you can see which of the linters have failed directly looking at the failed workflow step
peb-adr
left a comment
There was a problem hiding this comment.
Formatting looks good and sensible.
I approve although rrenkert has remarks on make targets.
peb-adr
left a comment
There was a problem hiding this comment.
Sigh, this comment wasn't saved to the last review..
|
|
||
| cleanup-yaml: | ||
| docker build -t meta-prettier-image -f Dockerfile-prettier . | ||
| docker run -d --name meta-prettier-container -v ..:/app/ meta-prettier-image |
There was a problem hiding this comment.
You can run --rm, obsoleting docker rm after docker stop.
From docker run --help
--rm Automatically remove the container and its associated anonymous volumes when it exits
No description provided.