search

impy-project / chromo

Hadronic Interaction Model interface in PYthon
Other
30 stars 7 forks source link

Fix workflow file 2 #149

Closed jncots closed 1 year ago

jncots commented 1 year ago

This is a second trial to launch PyPI. There is a problem with conditions. It seems that github.event.base_ref is null. At least this is what it shows when the workflow was launched on a fork. On my test repository github.event.base_ref has correct value. I tested conditions on a fork of chromo and made a release. It worked. I hope this time it should work.

HDembinski commented 1 year ago

I use contains(github.event.ref, '/tags/'), see https://github.com/scikit-hep/pyhepmc/blob/main/.github/workflows/wheels.yml#L83 Your condition may also work. I wonder why you not just follow pyhepmc. I made an effort make the CI code in pyhepmc as simple as possible.

Other suggestions for a follow-up PR, which should clean up the code a little bit:

Remove wheels_on_PR to simplify the code. We don't need to automatically check whether generating wheels still works after each PR, because most PRs won't affect the wheels. I first thought that you want to generate "nightlies" automatically, but then you should generate wheels for all platforms, so use the step "wheels" instead of "wheels_for_PR".

In all my repositories, generating the wheels just works, unless I actually touch the CI code. And in that case, one can trigger the workflow manually to check that wheels still work.

Remove name: tags. They are often useless and overused. They fill a purpose like comments in code. In style guides for comments in code, it is discouraged to do something like this

def evaluate(x):  # evaluates x
    ...

because the comment does not add any value, it is just noise. It is the same with Github CI scripts, where I often see superfluous name: tags, also in otherwise fine projects. That is nonsense that should not be copied (avoid "cargo cult"). Examples where the name is redundant:

For context, I think that you have places where name: tags are useful, where they serve as comments what the following step is doing. The tag is useful when that is not immediately obvious from the executed commands. This is good use: https://github.com/impy-project/chromo/blob/2cd6cb8c38efa1a32a736ba5357bbac34954ea55/.github/workflows/release.yml#L31 And everywhere in wheels. This list is not complete, I just want to give some examples for good use.

jncots commented 1 year ago

Thank you @HDembinski for the comment. I will improve the workflow file in next PRs.