search

SolomonDefi / solomon-monorepo

Monorepo containing core Solomon apps, services, libraries, and deploy config.
6 stars 3 forks source link

Python app formatter issue #153

Closed solomondefi-dev closed 3 years ago

solomondefi-dev commented 3 years ago

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: https://github.com/nrwl/nx/issues/2960

kelvin-wong commented 3 years ago

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 commented 3 years ago

@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.

solomondefi-dev commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 1.3.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: