-
Notifications
You must be signed in to change notification settings - Fork 28
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
Update required CI checks for rust-libp2p #73
Conversation
Companion PR for libp2p/rust-libp2p#3090.
6fe4789
to
1e4e7e4
Compare
Before merge, verify that all the following plans are correct. They will be applied as-is after the merge. Terraform planslibp2p
|
Oh wow, is a499ace some kind of auto-formatting bot? Interesting. I didn't make that commit :) |
Yes! We can define custom actions which can further modify the YAML config whenever it changes. They are defined in https://github.com/libp2p/github-mgmt/blob/master/scripts/src/actions/fix-yaml-config.ts. As you can see, one of them is I see the commit is associated with you rather than with the bot account. I'll have a look at it when I have a moment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for following this all the way through @thomaseizinger! Just one question.
- rustfmt | ||
- Test libp2p-autonat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand correctly that each time we add a new crate we have to touch this file? I am assuming there is no out-out instead of opt-in for required status checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, status checks are not required so we don't need to touch the file unless we want them to be required!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Thanks @thomaseizinger. And also thanks @libp2p/ipdx for codifying these configurations!
If I am not mistaken, this should only be merged once libp2p/rust-libp2p#3090 is merged. |
It is the other way round. We have to merge this first so we can even merge libp2p/rust-libp2p#3090 (unless you force-merge that one with admin priviledges). Once this is merged, nothing else can merge apart from libp2p/rust-libp2p#3090. |
@mxinden libp2p/rust-libp2p#3090 is good to merge. Can we merge here? (I don't have the rights to merge.) |
Summary
Companion PR for libp2p/rust-libp2p#3090.
Why do you need this?
What else do we need to know?
DRI: myself
Reviewer's Checklist