pcdshub / lightpath

LCLS Lightpath Module
https://pcdshub.github.io/lightpath
Other
4 stars 9 forks source link

DEV/MNT: migrate to latest standards with pyproject.toml [LCLSPC-603] #175

Closed tangkong closed 1 year ago

tangkong commented 1 year ago

Description

Runs pcds-python-migration-tools:update_repository.py from https://github.com/pcdshub/pcds-python-migration-tools/pull/3

Motivation and Context

Trying out GHA with Ken's spiffy new migration tools

Bits that weren't immediately taken care of by the migration tool (migration tool has been updated since, taking care of some of these problems)

How Has This Been Tested?

testing here?

Where Has This Been Documented?

documentation here?

klauer commented 1 year ago

For those conda build failures, nuke your build.sh and fix meta.yaml to have these lines:

https://github.com/pcdshub/cookiecutter-pcds-python/blob/72f80d77e5bf13d2e5b4a3d0343fde4685688bda/%7B%7B%20cookiecutter.folder_name%20%7D%7D/conda-recipe/meta.yaml#L1-L16

I think I want to add that to the migration tool, too...

klauer commented 1 year ago

I used your notes to fix up the migration tool a bit more - thanks!

One remaining thing: add setuptools_scm and pip as build deps in the conda recipe

Rest is probably good 🤞

tangkong commented 1 year ago

3.11 Failures:

tangkong commented 1 year ago

Although the 3.11 tests are marked as experimental and do not prevent the overal test job from failing, the little status icon next to the commit hash still suggests failure. :/

It could just be something to get used to, but I didn't uncover anything during my brief search

image
klauer commented 1 year ago

Although the 3.11 tests are marked as experimental and do not prevent the overal test job from failing, the little status icon next to the commit hash still suggests failure. :/

This needs to be configured here in the branch protection settings:

image

It's sensible from GitHub's standpoint: users might want to have different rules depending on the branch being merged into. And that shouldn't be baked into the actions configuration itself.

Since we want a generally consistent set of settings., I'm not sure if there's a better or easier way for us to configure each repository.

klauer commented 1 year ago

For 3.11's test failure, it's not typhos's fault entirely:

In [1]: from ophyd.ophydobj import Kind

In [2]: list(Kind)
Out[2]: [<Kind.normal: 1>, <Kind.config: 2>]

Looks to be an ophyd bug or incompatibility with 3.11's changes

tangkong commented 1 year ago

I suppose I expected the experimental setting to affect this, but in our case since fail_fast is false it doesn't affect the workflow.

Having to manually set up each repository's check failure settings is annoying, and adding another moving piece to update when our ci helpers change sounds bad to me.

klauer commented 1 year ago

Having to manually set up each repository's check failure settings is annoying, and adding another moving piece to update when our ci helpers change sounds bad to me.

Completely agreed.

I've found that the GitHub API can be used to list/add/delete branch protection rules, meaning we could automate the procedure. If that ends up not working well, I think it'll be time to re-evaluate the job matrix.

tangkong commented 1 year ago

I'll re-request Ken's review, though nothing much has changed since he last looked at the repo. Truly I really just crave validation