search

bluesky / hklpy

Diffractometer computation library with ophyd pseudopositioner support
https://blueskyproject.io/hklpy
BSD 3-Clause "New" or "Revised" License
3 stars 11 forks source link

repair failing unit test #211

Closed prjemian closed 2 years ago

prjemian commented 2 years ago
prjemian commented 2 years ago

Is it necessary to run black, flake8, & pytest in the publish_docs workflow? These are out of scope of this workflow. Should not tag (which would actually publish the docs built here) if these processes have failed the unit test or linting worfkflows.

https://github.com/bluesky/hklpy/blob/54defeaabb441bb554df8b6baeccda15342ae24e/.github/workflows/publish-docs.yml#L76

https://github.com/bluesky/hklpy/blob/54defeaabb441bb554df8b6baeccda15342ae24e/Makefile#L5

prjemian commented 2 years ago

@mrakitin This is ready for review. I assumed that the gobject-introspection package requirement would be inherited from the hkl package. This PR enforces that requirement. The added Diagnotics in the worfklow are useful to identify the problem more quickly.

mrakitin commented 2 years ago

Is it necessary to run black, flake8, & pytest in the publish_docs workflow? These are out of scope of this workflow. Should not tag (which would actually publish the docs built here) if these processes have failed the unit test or linting worfkflows.

https://github.com/bluesky/hklpy/blob/54defeaabb441bb554df8b6baeccda15342ae24e/.github/workflows/publish-docs.yml#L76

https://github.com/bluesky/hklpy/blob/54defeaabb441bb554df8b6baeccda15342ae24e/Makefile#L5

Maybe we could use the needs feature of the workflows to enable chained conditional execution, as we did in https://github.com/NSLS-II/edrixs/blob/fc1a316df2532950b73325e501b41a7cd37ae80e/.github/workflows/ci-test.yml#L26? It'd look like: https://github.com/NSLS-II/edrixs/actions/runs/1492481471.

prjemian commented 2 years ago

@mrakitin Thanks for the quick review. The chained workflow example using needs is a great suggestion (wondered how to do that). That chained workflow puts the entire workflow into one YAML file.

I'll merge this PR and study implementation in a test project before introducing a chained workflow here.

mrakitin commented 2 years ago

@mrakitin Thanks for the quick review. The chained workflow example using needs is a great suggestion (wondered how to do that). That chained workflow puts the entire workflow into one YAML file.

Yes, that was a shortcoming. I tried to see how this can be split into separate files and that are included in the main workflow, but it apparently requires writing separate actions, which is much more work than just extracting it into a separate file. With Azure Pipelines it's done much more trivially (see e.g., https://github.com/NSLS-II/profile-collection-ci/blob/418b045897dd8cb8bc2a7013a70e18f30bc8006d/nsls2-collection-2021-3.0-py39.yml#L49).