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

ci: Change xurls url #836

Closed
wants to merge 1 commit into from

Conversation

chavafg
Copy link
Contributor

@chavafg chavafg commented Oct 22, 2018

xurls repo url is not being resolved. Switch to
the github url.

Fixes: #835.

Signed-off-by: Salvador Fuentes salvador.fuentes@intel.com

xurls repo url is not being resolved. Switch to
the github url.

Fixes: kata-containers#835.

Signed-off-by: Salvador Fuentes <salvador.fuentes@intel.com>
@chavafg
Copy link
Contributor Author

chavafg commented Oct 22, 2018

/test

1 similar comment
@chavafg
Copy link
Contributor Author

chavafg commented Oct 23, 2018

/test

@caoruidong
Copy link
Member

caoruidong commented Oct 23, 2018

LGTM. This is very urgent.

Approved with PullApprove

@jodh-intel
Copy link
Contributor

jodh-intel commented Oct 23, 2018

Raised kata-containers/ci#74 for the ARM CI failure (which appears unrelated to this PR).

lgtm

Approved with PullApprove

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

Weird. I saw this failure a week or two ago, tried that URL change fix locally, and it did not work....
Given most of the CIs seem happy....
lgtm

@@ -385,7 +385,7 @@ check_docs()
if [ ! "$(command -v $cmd)" ]
then
info "Installing $cmd utility"
go get -u "mvdan.cc/xurls/cmd/$cmd"
go get -u "github.com/mvdan/${cmd}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah - actually, this needs to be:

Suggested change
go get -u "github.com/mvdan/${cmd}"
go get -u "github.com/mvdan/xurls/cmd/${cmd}"

Copy link
Contributor

Choose a reason for hiding this comment

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

When I saw this last week, I could not make the github URL work. The 'mvdan.cc' site seemed to be temporarily down, and then came back up, and everything started working for me again, hence I didn't file a fix or dig deeper on how to make the github URLs work.

@jodh-intel
Copy link
Contributor

We can't merge this until the xurls URL is corrected (oh the irony... 😄)

@jodh-intel
Copy link
Contributor

... and the reason the CI tests passed here is that the xurls command is only used when the PR changes any documentation files.

So although go get -u "github.com/mvdan/xurls "works", it doesn't install the xurls binary. For that, you need to go get -u "github.com/mvdan/xurls/cmd/xurls.

@jodh-intel
Copy link
Contributor

In summary, this fix is urgent, but only new PRs that change documents will fail the CI stage until this PR lands.

@jodh-intel
Copy link
Contributor

jodh-intel commented Oct 23, 2018

... errmm. Correction:

  • all new PRs will fail as the existing URL for xurls is now invalid.
  • all new PRs that are raised subsequent to this PR landing will only fail if they contain documentation.

@caoruidong
Copy link
Member

Yes. All PRs are blocked now.

@jodh-intel
Copy link
Contributor

I'll raise a PR that replaces this one so we can resolve this "now"...

@jodh-intel
Copy link
Contributor

Closing as replaced by #839.

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.

xurls repo url is not working
4 participants