Skip to content

Conversation

crazy-max
Copy link
Contributor

follow-up #3634 #3640

remove dco check that is already done by probot and move test step from ci workflow to build workflow and makes build job depends on it so we make sure tests passed before building distribution. tests are not sandboxed yet in our Dockerfile but could do that in a follow-up.

@codecov-commenter
Copy link

codecov-commenter commented May 5, 2022

Codecov Report

Merging #3644 (b066451) into main (8794122) will increase coverage by 0.80%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3644      +/-   ##
==========================================
+ Coverage   56.58%   57.38%   +0.80%     
==========================================
  Files         103      105       +2     
  Lines        7520    10832    +3312     
==========================================
+ Hits         4255     6216    +1961     
- Misses       2596     3932    +1336     
- Partials      669      684      +15     
Impacted Files Coverage Δ
...istribution/distribution/registry/storage/error.go
...tribution/registry/storage/blobwriter_resumable.go
...istribution/distribution/registry/handlers/blob.go
...ribution/distribution/registry/storage/registry.go
...bution/distribution/registry/storage/filereader.go
...ution/distribution/registry/api/errcode/handler.go
...n/distribution/registry/storage/driver/fileinfo.go
...ution/distribution/registry/storage/driver/walk.go
...hub.com/distribution/distribution/context/trace.go
...on/registry/client/auth/challenge/authchallenge.go
... and 198 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8794122...b066451. Read the comment docs.

@crazy-max crazy-max force-pushed the cleanup-ci branch 2 times, most recently from ae72e00 to aa5bf43 Compare May 5, 2022 16:10
@milosgajdos milosgajdos requested a review from thaJeztah May 6, 2022 08:10
@milosgajdos
Copy link
Member

CC: @thaJeztah

fi

verbosity="${DCO_VERBOSITY--v}"
GIT_CHECK_EXCLUDE="./vendor:./script/validate/template" git-validation "$verbosity" -range "$COMMIT_RANGE" -run DCO,short-subject,dangling-whitespace
Copy link
Member

Choose a reason for hiding this comment

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

The old script was also validating for dangling whitespaces in the commit messages, and for a maximum subject length; do we have a replacement for that?

Copy link
Contributor Author

@crazy-max crazy-max Jun 16, 2022

Choose a reason for hiding this comment

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

Added it back the sandboxed way so we are aligned with the rest of our validation process. See the respective commit.

Copy link
Member

Choose a reason for hiding this comment

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

Wondering now; if we keep the script, should we keep the DCO in the dockerfile check as well (to allow a local validate)?

Not a blocker for me, but wondering if it's worth keeping, as it's only a extra check in that stage now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think we need a local validation for DCO imo.

@crazy-max crazy-max force-pushed the cleanup-ci branch 3 times, most recently from b9012da to f5f3c98 Compare June 16, 2022 15:09
@crazy-max crazy-max requested a review from thaJeztah June 16, 2022 15:12
@crazy-max crazy-max mentioned this pull request Jul 20, 2022
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max requested a review from thaJeztah July 20, 2022 14:51
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@milosgajdos milosgajdos merged commit df14ebe into distribution:main Jul 21, 2022
@crazy-max crazy-max deleted the cleanup-ci branch July 21, 2022 08:36
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.

4 participants