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

.github: add new ci_release workflow to unify CI #1191

Merged
merged 9 commits into from
Oct 13, 2022

Conversation

MSevey
Copy link
Member

@MSevey MSevey commented Sep 30, 2022

Edit: Testing some things.

Overview

The topic of releases came up in a recent weekly retro. This PR has some suggested updates of how to improve the release process and DRY up the GitHub actions. Below are some key takeaways from the changes.

  • new make cover command
  • use of composite github actions
  • use of reusable github workflows
  • common CI file for PRs and release

make cover

This is a simple change to move the coverage report logic into the local Makefile so that coverage reports can be made locally as well as on the online CI.

Composite GitHub Actions

Composite GitHub actions are great 😄
They allow for a nice way to refactor common actions. These can be used internal to a repo and external to a repo. In this change you can see the yamllint stage of the CI is a composite action that is stored in the celestiaorg/.github repo. This means all Celestia repos can use the same yamllint action to ensure consistent yaml file linting organization wide.

The markdown-lint stage is another good candidate to be refactored into the celestiaorg/.github repo so that all markdown files are consistent.

Read more here: https://docs.github.com/en/actions/creating-actions/creating-a-composite-action

Reusable GitHub Workflows

Reusable workflows are similar to composite actions. Both can be used and have different trade offs. In practice it seems more dealers choice as to which you use.

The hadolint job is an example of a reusable workflow for linkting of dockerfiles. This PR also moved the go-ci and the docker-build workflows to be reusable to not change any core logic.

Read more here https://docs.github.com/en/actions/using-workflows/reusing-workflows

Common CI

The benefit of having a singular common CI for PRs and releases is to ensure that we are always linting, testing, and building our code the same way and not skipping steps for different cases, which can lead to bugs getting released.

NOTE: This change will also change the team's workflow in how they are doing releases. Instead of doing a new tag locally, the process will be to manually trigger this workflow from the actions tab on this repo. You will see a drop down for what type of version you are releasing (patch, minor, major).

Notes

@renaynay can you give me write access? I don't have write access so had to fork and push.

@Wondertan the updates to the github actions that this PR makes is what I was referring to on the retro call today.

@MSevey
Copy link
Member Author

MSevey commented Sep 30, 2022

I think the reason my github action updates are running is because it is a fork plus i don't have write access.
Once i have write access i can resubmit.

@m-kus
Copy link

m-kus commented Oct 2, 2022

Hey!
It would be cool to have semver docker tags for the celestia-node image, I noticed that this PR touches the docker-build workflow so decided to first write here :)
It will require triggering the docker-build workflow on git tags and adding type=semver,pattern={{version}} to the Docker meta step.
Pls let me know if it's ok and whether it should be done in a separate PR.

@MSevey
Copy link
Member Author

MSevey commented Oct 3, 2022

Hey! It would be cool to have semver docker tags for the celestia-node image, I noticed that this PR touches the docker-build workflow so decided to first write here :) It will require triggering the docker-build workflow on git tags and adding type=semver,pattern={{version}} to the Docker meta step. Pls let me know if it's ok and whether it should be done in a separate PR.

We can easily do that, in fact I already have some composite actions here that do the the semver releases https://github.com/celestiaorg/.github/tree/main/.github/actions/major-minor-version

I wanted to talk with the team first about the impacts of that change before making them. Didn't want to change the flow too much :-)

@Wondertan
Copy link
Member

@MSevey, wen ready for review?

@MSevey MSevey force-pushed the sevey/github-action-updates branch from 42749cf to 5c05c99 Compare October 5, 2022 19:58
@MSevey
Copy link
Member Author

MSevey commented Oct 5, 2022

@MSevey, wen ready for review?

@renaynay I mostly created this PR as a test to play around with some things and show an example of what I was talking about during the retro around release. I'll start a convo in slack with the Team to see if this is something we would like to close out. If not it is totally fine to push out and close this for now.

@MSevey MSevey marked this pull request as ready for review October 7, 2022 16:58
@MSevey
Copy link
Member Author

MSevey commented Oct 7, 2022

Reverted docker-build changes as there were permission issues. Ultimately the docker builds can be common in the celestiaorg/.github repo so it isn't worth fixing the permission issues here.

@codecov-commenter
Copy link

codecov-commenter commented Oct 7, 2022

Codecov Report

Merging #1191 (382c240) into main (793e2b0) will increase coverage by 0.28%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1191      +/-   ##
==========================================
+ Coverage   55.55%   55.84%   +0.28%     
==========================================
  Files         160      160              
  Lines        9853     9853              
==========================================
+ Hits         5474     5502      +28     
+ Misses       3841     3815      -26     
+ Partials      538      536       -2     
Impacted Files Coverage Δ
fraud/pb/proof.pb.go 36.61% <0.00%> (+1.89%) ⬆️
fraud/bad_encoding.go 66.00% <0.00%> (+3.00%) ⬆️
share/ipld/get.go 92.88% <0.00%> (+4.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@MSevey MSevey force-pushed the sevey/github-action-updates branch from df79759 to 119c080 Compare October 7, 2022 17:56
@MSevey
Copy link
Member Author

MSevey commented Oct 7, 2022

Ready for review 👍

Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Mostly questions. Thanks for tidying this up!

.github/workflows/ci_release.yml Show resolved Hide resolved
.github/workflows/ci_release.yml Show resolved Hide resolved
docker/Dockerfile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@MSevey MSevey requested a review from Wondertan October 11, 2022 14:36
@MSevey MSevey merged commit 30ed57f into celestiaorg:main Oct 13, 2022
@MSevey MSevey deleted the sevey/github-action-updates branch October 13, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:testing Related to unit tests
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants