search

CHLNDDEV / oceanmesh

Automatic coastal ocean mesh generation in Python and C++. **under development**
GNU General Public License v3.0
51 stars 15 forks source link

fix for CI (black) #78

Closed tomsail closed 5 months ago

tomsail commented 6 months ago

To solve #77

I am first putting as draft to if CI passes

krober10nd commented 5 months ago

Seems like there were some issues with starting up the CI system with versions of Python other than 3.9?

pmav99 commented 5 months ago

The reason this fails is because you are asking black to automatically fix the issues it finds and it seems that there are permissions issues when it tries to amend the commits. You can either:

  1. Try to fix the permissions issues: https://github.com/CHLNDDEV/oceanmesh/actions/runs/8567158490/job/23478362132?pr=78#step:5:89
  2. Use a flag like black --check or black --diff which will fail if there are differences without trying to make changes to the source code. Not sure how you do that with wearerequired/lint-action@master but the docs should mention something.
tomsail commented 5 months ago

So there are various problems here:

  1. the formatter CI: the permissions issue with wearerequired/lint-action@master does not work for forks. Issue is still open on wearerequired page. We might want to test in a different way.
  2. sudo apt install -y libopenmpi3 libopenmpi-dev openmpi-bin failing for some python versions. example here
  3. black version issue. I will try to see which version throws the least number of errors. I had started to test (as you can see in the commit), but didn't finish it.
tomsail commented 5 months ago

the formatter CI: the permissions issue with wearerequired/lint-action@master does not work for forks. Issue is still https://github.com/wearerequired/lint-action/issues/13 on wearerequired page. We might want to test in a different way.

It works anyway if black and flake go through. Although permissions are still an issue (see last logs), if linting is fixed it won't raise any error at the end.

sudo apt install -y libopenmpi3 libopenmpi-dev openmpi-bin failing for some python versions. example here

Fixed by adding "sudo apt update in the CI steps"

black version issue. I will try to see which version throws the least number of errors. I had started to test (as you can see in the commit), but didn't finish it.

Fixed by removing: 

line-length = 108

and letting default maximum line-length

tomsail commented 5 months ago

should be all good now. let's wait to see if latest CI pass

krober10nd commented 5 months ago

Excellent thank you Thomas.

tomsail commented 5 months ago

@krober10nd is it really necessary to have the formatter.yml CI action? It seems a bit overkill to use and deploy wearerequired/lint-action@master (which is itself a library that can induce changes) for just the linting (that we already have in the test CI action.

krober10nd commented 5 months ago

it is not essential since the PR author will know if it is violating the format. I'm in favor of removing it.