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

ci: uci/copy-templates #283

Merged
merged 16 commits into from
Jul 12, 2024
Merged

ci: uci/copy-templates #283

merged 16 commits into from
Jul 12, 2024

Conversation

web3-bot
Copy link
Collaborator

@web3-bot web3-bot commented Jul 1, 2024

This PR was created automatically by the @web3-bot as a part of the Unified CI project.

Closes #282

@galargh galargh marked this pull request as draft July 1, 2024 14:55
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@rvagg
Copy link
Member

rvagg commented Jul 2, 2024

#284 deals with some of this

@laurentsenta laurentsenta force-pushed the uci/copy-templates branch 4 times, most recently from a7b0ab7 to c557500 Compare July 2, 2024 08:16
@laurentsenta
Copy link
Contributor

laurentsenta commented Jul 2, 2024

Double checking make gen check: failed job

@laurentsenta laurentsenta force-pushed the uci/copy-templates branch 2 times, most recently from 37c6142 to aafc97b Compare July 2, 2024 08:20
@laurentsenta
Copy link
Contributor

laurentsenta commented Jul 2, 2024

Thanks for sharing the patch, @rvagg. I rebased on top of your work,

We'll skip testing for race conditions for now,

Do you care about testing on Windows and macOS?

They come for free with UCI, but it's a bit noisier to have four more CI checks, and Windows testing seems much slower (6+ minutes here).

@laurentsenta laurentsenta requested a review from galargh July 2, 2024 08:30
@rvagg
Copy link
Member

rvagg commented Jul 2, 2024

Do you care about testing on Windows and macOS?

macOS yes, Windows not so much, we can skip for now I think

Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we deprecate CircleCI in one Go? I believe we do have feature parity so I think we're good to go.

@galargh galargh marked this pull request as ready for review July 2, 2024 12:24
@rvagg
Copy link
Member

rvagg commented Jul 3, 2024

I think we should hold off on this until we have v0.14 squared away unfortunately. There's a lot of churn in here and the go.mod version bump is not ideal to push through while we're doing RCs.

But otherwise I'm excited to get this landed.

@BigLep
Copy link
Member

BigLep commented Jul 9, 2024

I think we should hold off on this until we have v0.14 squared away unfortunately. There's a lot of churn in here and the go.mod version bump is not ideal to push through while we're doing RCs.

@rvagg : when are you thinking we could pick this up? Once we have a final-for-mainnet release for Lotus (v1.28.0)?

I'm not saying you're wrong, but what is the biggest risk you're seeing here? I thought this PR was just automating a lot of the steps that we do manually (and without review) in the GitHub UI. Is the worst case that we publish a botched version? Wouldn't we only update Lotus to depend on it when we have a good version?

@rvagg
Copy link
Member

rvagg commented Jul 10, 2024

@BigLep it's not a big deal, but go.mod bumps from 1.18 to 1.21. So a breaking change. There's also a lot of other fixes in here that add churn. But then again .. holding this off would make it a breaking change and would make patch releases harder post v14! So perhaps we should just bundle it all in, like I'm advocating for the BatchReturn change.

@BigLep
Copy link
Member

BigLep commented Jul 11, 2024

@laurentsenta: @rvagg has verbally approved this. He was holding off approval in the tool because of uncertainty on whether to merge this into master now given the ongoing Network release/upgrade. We talked this through and he's good with this change.

Also, this effectively closes #275 as well right?

@rvagg
Copy link
Member

rvagg commented Jul 12, 2024

Yes, good to go, I ran this on top of master and #288 on top of this during the calibnet upgrade to give us an additional level of assurance of what we think is likely to be a v14 final.

@galargh galargh merged commit ac23f07 into master Jul 12, 2024
8 of 9 checks passed
@galargh galargh deleted the uci/copy-templates branch July 12, 2024 13:16
@galargh
Copy link
Contributor

galargh commented Jul 12, 2024

Merged, updated required checks and stopped building in Circle CI ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

Migrate CI from CircleCI to GitHub Actions
7 participants