Billingegroup / cookiecutter

A cookie-cutter for DiffPy packages.
Other
1 stars 8 forks source link

Fix CI docs.yml that works for diffpy.snmf #104

Closed bobleesj closed 4 weeks ago

bobleesj commented 1 month ago

Bugs were found when CI testing diffpy.snmf docs with the recent updated made in the following commit in the cookiecutter repo for the workflows/docs.yml file https://github.com/Billingegroup/cookiecutter/commit/c7d98cf8ffd057a93749ccc55ea2e9ef5a43c839.

References:

sbillinge commented 1 month ago

The only question I have here before we update the cookiecutter is to understand why it worked before but stopped working in the latest iteration.

I think it may be because we added the code for running tests and uploading coverage, though I think I used this same cookiecutter for releasing diffpy.structure.

Any thoughts anyone?

Tieqiong commented 1 month ago

The only question I have here before we update the cookiecutter is to understand why it worked before but stopped working in the latest iteration.

I think it may be because we added the code for running tests and uploading coverage, though I think I used this same cookiecutter for releasing diffpy.structure.

Any thoughts anyone?

@sbillinge I think it was different in diffpy.structure. The codecov test, upload, and secret were added in main.yml but not docs.yml. So the docs.yml file used to be purely for building and deploying document (which reflects on the environment - the environment was not built for testing). Now we add run tests and upload coverage in docs.yml. This could be a good action as we don't need to upload to codecov every time we push to main. However if we want to do this when making docs we have to make sure the testing is set up in the docs workflow.

sbillinge commented 1 month ago

I wonder if we actually want to have a different action for this, so there is a docs action and a codecov action? It would be clearer in the future what is going on?

Tieqiong commented 1 month ago

It would be good for a clearer workflow. By making a new yml the workflow is more modular and easier to manage, decouple with main and docs so that we can have a better trigger.

However I can imagine it increase the human effort during the workflow (e.g. we make a codecov branch and set the trigger when we push to this branch, but sometimes we forget and it get out of sync). Another problem is we are having duplicated steps (e.g. running test on both main and codecov, which is also happening now). It's not a huge problem but I still feels we can make it better...

sbillinge commented 1 month ago

O don't quite see that it increases complication. It is all automated.

Tieqiong commented 1 month ago

I don't know if I'm thinking correctly but like we want to add a good trigger for the CI. We can let it act when we push to main, which will indeed be automated (in the sense that we don't need to consider when to upload to codecov). However why not make it into main then? We want to trigger codecov upload when we want to, e.g. make a codecov branch and push to codecov when we want to update codecov. However by adding this flexibility we need to at least do something manually (e.g. push to branch codecov). That's what I mean by saying increase complication...

sbillinge commented 1 month ago

We could do it on push to main or on release, whichever behavior we prefer

Tieqiong commented 1 month ago

Yes, a separate yaml with release trigger could be a better choice if we just want to make the work easier, even though the new yml file might be mostly the same as main.yml.

However from the perspective of making sure we have good code, it's better to integrate codecov report into main.yml so that we can often see on the readme badge about the test coverage percentage. It also avoid the possibility of accidentally having very low codecov score/non working codecov badge for a specific release, which could be there for years.