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

Python app formatter issue #153

Closed
solomondefi-dev opened this issue Nov 5, 2021 · 7 comments · Fixed by #157
Closed

Python app formatter issue #153

solomondefi-dev opened this issue Nov 5, 2021 · 7 comments · Fixed by #157
Assignees
Labels

Comments

@solomondefi-dev
Copy link
Collaborator

Enable Solomon formatter python via pnpx nx format:check --all.

I’m not sure if this is possible, but it seems like the generic Nx format runner doesn’t pick up the python task, so running the Nx commands skips Black (and CI doesn’t work as expected). This may not be so easy to solve, and requires some research into Nx

Note - in CI, the command should usually be pnpx nx format:check --base=origin/main, but we don't have a way for Nx to detect that Python apps have changed. There is a hacky workaround implemented by the author of nx-plus for their Vue plugin, some discussion here: nrwl/nx#2960

@kelvin-wong
Copy link
Collaborator

Seems the root cause of this issue is that Nx build the dep-graph to detect the changeset. Nx only supports that in js. Probably the solution would be to check the whole Python project instead of only the changed part.

@kelvin-wong
Copy link
Collaborator

@solomondefi-dev
just found that pnpx format:check --all only run the prettier against all projects so all .py files are not checked.
To run the formatter, pnpx run-many --target=format --all which run format command against all projects.

@kelvin-wong kelvin-wong self-assigned this Nov 8, 2021
@solomondefi-dev
Copy link
Collaborator Author

Will that work for CI? In that situation, we don't need the files to actually be formatted, just to fail if anything is unformatted.

@kelvin-wong
Copy link
Collaborator

need to add a --check option to black https://github.com/samatechtw/nx-python-poetry/blob/9c82851042e8d92edecbd79176741ff484528229/packages/fastapi/src/executors/format/executor.ts#L26

@solomondefi-dev
Copy link
Collaborator Author

@kelvin-wong The Nx python plugin is updated in the repo now, so --check should work. Can you try to add it to CI? It may be ok to keep the existing format check command that only runs on modified projects, and specifically run nx run api-dispute:format --check and nx run api-evidence:format --check every time, since they're pretty fast. What do you think?

@beefho67
Copy link
Collaborator

beefho67 commented Nov 9, 2021

Hey team! Please add your planning poker estimate with ZenHub @kelvin-wong @solomondefi-dev

@github-actions
Copy link

🎉 This issue has been resolved in version 1.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

3 participants