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

Strict name matching for Repository.GetTagID() #8074

Merged
merged 4 commits into from Sep 4, 2019

Conversation

@guillep2k
Copy link
Member

commented Sep 4, 2019

Fixes #7985

I'm assuming "\n" is OK for all platforms? Anyone running Mac?

guillep2k added 3 commits Sep 4, 2019
@guillep2k

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

Once again I had to use:

make clean generate swagger-check swagger-validate vendor

Then commit, push. Let's see what happens with CI now.

@GiteaBot GiteaBot added the lgtm/need 2 label Sep 4, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Sep 4, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@d9545f9). Click here to learn what that means.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #8074   +/-   ##
=========================================
  Coverage          ?   41.78%           
=========================================
  Files             ?      481           
  Lines             ?    64425           
  Branches          ?        0           
=========================================
  Hits              ?    26917           
  Misses            ?    34038           
  Partials          ?     3470
Impacted Files Coverage Δ
modules/git/repo_tag.go 60.1% <83.33%> (ø)

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 d9545f9...0e09148. Read the comment docs.

@guillep2k

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

So, I don't know why I had to update vendor/, as it is unrelated to this PR. Good news is that it works.

@lunny

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Could you add some tests for that? And is the depends libraries update is necessary?

@guillep2k

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

Could you add some tests for that? And is the depends libraries update is necessary?

@lunny I'll add a test, but the CI is failing again if I don't update vendor.

@guillep2k

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

@lunny see #8075 about the CI failing (it's a dummy PR with no real changes from master).

@@ -105,9 +105,9 @@ require (
golang.org/x/crypto v0.0.0-20190829043050-9756ffdc2472
golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
golang.org/x/sys v0.0.0-20190902133755-9109b7679e13
golang.org/x/sys v0.0.0-20190904005037-43c01164e931

This comment has been minimized.

Copy link
@typeless

typeless Sep 4, 2019

Contributor

Why does it have to be updated?

This comment has been minimized.

Copy link
@guillep2k

guillep2k Sep 4, 2019

Author Member

@typeless Only because of CI failing again. It's not this PR, is all of them.
For example: #8040 --> https://drone.gitea.io/go-gitea/gitea/12635

This comment has been minimized.

Copy link
@guillep2k

guillep2k Sep 4, 2019

Author Member

@typeless please see #8076 - that PR contains only the vendor update.

@lafriks lafriks added the kind/bug label Sep 4, 2019
@lafriks lafriks added this to the 1.10.0 milestone Sep 4, 2019
@lafriks
lafriks approved these changes Sep 4, 2019
@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels Sep 4, 2019
@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Sep 4, 2019
@lafriks lafriks merged commit 4cb1bdd into go-gitea:master Sep 4, 2019
2 checks passed
2 checks passed
approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details
@lafriks

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Please send backport to 1.9

@guillep2k guillep2k deleted the guillep2k:fix-7985 branch Sep 5, 2019
@6543

This comment has been minimized.

Copy link
Member

commented Oct 6, 2019

@guillep2k add label:backport/done ?

@lunny lunny added the backport/done label Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.