Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak behaviors of build and release CI workflows #847

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

altheaden
Copy link
Contributor

Description

Hello! With this PR I've tweaked some settings that @xylar and I have determined work out better in CI workflows on other repos. I've made the same changes here. These changes include:

  1. cancel_others is now set to false by default, which should make it easier to decipher if a bug is isolated to a specific version of python via CI
  2. All references to mamba/ Mambaforge have been removed and/or changed to conda / Miniforge3
  3. The timeout for the pre-commit job has been upped to 5 minutes (we were having some timeout issues during the caching step)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@xylar
Copy link
Contributor

xylar commented Sep 13, 2024

@altheaden, it looks like cancel others is still on here. Do you want to change that, too? Then, we can ask @tomvothecoder and @chengzhuzhang what they think of the changes in general.

@altheaden
Copy link
Contributor Author

@xylar Whoops, definitely a mistake, thanks for catching that! Fixed.

Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@altheaden, thanks for the latest change. This looks great to me!

@tomvothecoder, @forsyth2 and @chengzhuzhang, please have a look when you can. The change from Mambaforge to Miniforge3 should be straightforward because Mambaforge and the mamba command just aren't supported anymore as a separate thing (though they still exist for backwards compatibility).

We had some recent mysterious failing CI and it took us awhile to realize that caching can sometimes take more than 2 minutes so we suggest upping the timeout.

And finally, as @altheaden says above, we have found it more frustrating than helpful to have all the CI runs get cancelled when one fails. It's often good to know if the same problem persists across different builds or if it's specific to one (e.g. one version of python). We don't typically save a lot of cloud computing time either by canceling because the CI might not need to rerun if it were to succeed and it would typically fail around the same time as it gets cancelled if it were to fail.

@xylar
Copy link
Contributor

xylar commented Sep 13, 2024

One more note, if you're happy with these changes, @altheaden will make similar ones for zppy and zstash.

@forsyth2
Copy link
Collaborator

@altheaden Thanks for working on this!

@xylar From visual inspection alone, these changes seem fine for zppy and zstash too, thanks.

@xylar
Copy link
Contributor

xylar commented Sep 13, 2024

Great, let's make sure @tomvothecoder agrees since he wrote these workflows in the first place.

@tomvothecoder
Copy link
Collaborator

Hi @altheaden and @xylar, these changes look reasonable to me. I'm happy to have this PR merged. Thanks @altheaden!

@xylar
Copy link
Contributor

xylar commented Sep 13, 2024

@chengzhuzhang, it sounds like you can merge then if you're also happy.

@chengzhuzhang
Copy link
Contributor

It looks good to me. Thank you for this update @altheaden @xylar !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants