-
Notifications
You must be signed in to change notification settings - Fork 45
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
setup basic CI checks #10
Conversation
fbielejec
commented
Jun 12, 2023
•
edited
Loading
edited
- add clippy checks, fix all clippy warnings
- add toolchain file to set same rustc for all (set it to stable but happy to change)
- add fmt file with basic formatting options
- run fmt --all
- basic CI with caching, branch protection rules
This would probably cause a lot of conflicts with #9, could we run this once that is merged? |
You could just format that PR after a merge and resolve to yours? |
I created a basic action together with branch protection rules but I cannot set them, as we have a private repo on a free plan: The easiest would be to just turn it public, are there any reasons to keep it hidden @JoshOrndorff @notlesh WYT? |
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.
Excellent thank you!
Regarding making the repo public, I fully support it. I am actively advocating for making the entire academy content public. I haven't yet succeeded in convincing the admins to do that, so I guess it is up to @nukemandan to decide if we can make this repo public.
There is really nothing private in here at all. There aren't even instructions for what we will do in class. It is just a Substrate node. So I hope we can open it.
In any case, feel free to merge this when you are ready.
Yeah, let's not hold this up. I can own the process of resolving it with #9. |
you have my axe if help needed. ok merging |