Skip to content

Use actions v2 on nightly ci#4191

Closed
inductor wants to merge 3 commits intocontainerd:masterfrom
inductor:actions/checkout_v2
Closed

Use actions v2 on nightly ci#4191
inductor wants to merge 3 commits intocontainerd:masterfrom
inductor:actions/checkout_v2

Conversation

@inductor
Copy link
Copy Markdown

@inductor inductor commented Apr 16, 2020

The main CI uses checkout@v2 so I made nightly follows it!

inductor
inductor commented Apr 16, 2020
View reviewed changes
Comment thread .github/workflows/nightly.yml Outdated
@inductor inductor force-pushed the actions/checkout_v2 branch from 364cb54 to c779c6e Compare April 16, 2020 03:44
@mxpv
Copy link
Copy Markdown
Member

mxpv commented Apr 16, 2020

Could you please sign off your commits (try git commit -s) in order to pass CI checks?
https://github.com/containerd/project/blob/master/CONTRIBUTING.md#sign-your-work

@inductor inductor force-pushed the actions/checkout_v2 branch from c779c6e to 04270f8 Compare April 16, 2020 05:15
@inductor
Copy link
Copy Markdown
Author

inductor commented Apr 16, 2020

@mxpv Done. Thanks 🙏

@mxpv
Copy link
Copy Markdown
Member

mxpv commented Apr 16, 2020

Changing actions/checkout to v2 will change your current working directory, so all subsequent steps will fail. Could you please verify that this script works in your fork first?

AkihiroSuda
AkihiroSuda requested changes Apr 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Please sign the commit with your real name

@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Apr 29, 2020

ping @inductor

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented May 4, 2020

ping @inductor could you fix your sign-off?

@inductor inductor force-pushed the actions/checkout_v2 branch 2 times, most recently from e6640d8 to fed0626 Compare May 15, 2020 00:57
@mxpv mxpv mentioned this pull request May 15, 2020
Merged
@fuweid
Copy link
Copy Markdown
Member

fuweid commented May 15, 2020

Signed-off-by: inductor <[email protected]>

Sorry, could you sign your real name? Thanks!

@inductor inductor force-pushed the actions/checkout_v2 branch from fed0626 to c273142 Compare May 15, 2020 03:08
AkihiroSuda
AkihiroSuda approved these changes May 15, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda 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

@estesp
Copy link
Copy Markdown
Member

estesp commented May 15, 2020

I think the only review comment missed here is that @mxpv noted the issue with checkout v2 and working directory; any following step that needs/expects the "checkout dir" to be the working/PWD you have to set it explicitly, like here: https://github.com/containerd/containerd/blob/master/.github/workflows/ci.yml#L174

Hard to tell if that is going to break anything as I don't see any runs in your fork with this change that show if it is working or not. But, I think for example the make binaries command in the build actions may fail as the current working dir may not be set to the checkout location.

@mxpv
Copy link
Copy Markdown
Member

mxpv commented May 15, 2020

Please rebase this from master, that will run nightly job here so we can make sure everything is ok.

@containerd containerd deleted a comment from theopenlab-ci Bot May 15, 2020
@containerd containerd deleted a comment from theopenlab-ci Bot May 15, 2020
@containerd containerd deleted a comment from theopenlab-ci Bot May 15, 2020
@containerd containerd deleted a comment from codecov-io May 15, 2020
@containerd containerd deleted a comment from theopenlab-ci Bot May 15, 2020
@inductor inductor force-pushed the actions/checkout_v2 branch from c273142 to 3f7bf3c Compare May 16, 2020 13:01
@inductor
Copy link
Copy Markdown
Author

inductor commented May 16, 2020

@mxpv I just rebased!

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 16, 2020

Build succeeded.

@mxpv
Copy link
Copy Markdown
Member

mxpv commented May 16, 2020

@inductor So nightly builds are failing with make: *** No rule to make target 'binaries'. Stop. 11. I think you need to set a correct path for every build step.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 17, 2020

Build succeeded.

@inductor inductor force-pushed the actions/checkout_v2 branch from 64b0ea4 to 67193f4 Compare May 17, 2020 00:26
add workingdir at make
9ef08e3 
Signed-off-by: Kohei Ota <[email protected]>
@inductor inductor force-pushed the actions/checkout_v2 branch from 67193f4 to 9ef08e3 Compare May 17, 2020 00:27
@inductor
Copy link
Copy Markdown
Author

inductor commented May 17, 2020

Based on the error it also requires to install go dependencies. Am I doing this right? 🤔
I wonder how previous way worked.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 17, 2020

Build succeeded.

@mxpv
Copy link
Copy Markdown
Member

mxpv commented May 17, 2020

I guess GOPATH needs to be fixed as well.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 17, 2020

Build succeeded.

test
0760985 
Signed-off-by: Kohei Ota <[email protected]>
@inductor inductor force-pushed the actions/checkout_v2 branch from 96e9abb to 0760985 Compare May 17, 2020 02:45
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 17, 2020

Build succeeded.

@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Aug 3, 2020

@inductor Are you still interested in this?

@mxpv mxpv mentioned this pull request Oct 8, 2020
Merged
@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Oct 8, 2020

Closing in favor of #4612

@AkihiroSuda AkihiroSuda closed this Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@inductor @mxpv @AkihiroSuda @thaJeztah @fuweid @estesp