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

feat: add 'branches' git tag variant #7006

Merged
merged 1 commit into from
Jan 14, 2022
Merged

feat: add 'branches' git tag variant #7006

merged 1 commit into from
Jan 14, 2022

Conversation

snickell
Copy link
Contributor

@snickell snickell commented Jan 12, 2022

Fixes: #6989

Description
Implements a new git tagpolicy variant Branches, which can be used as follows:

tagPolicy:
  gitCommit:
    variant: 'Branches'

When this tagPolicy is active, when currently tracking a git branch, the branch's name (git branch --show-current) with slashes replaced with underscores will be used as the tag, otherwise the current short sha commit will be used.

@codecov
Copy link

codecov bot commented Jan 12, 2022

Codecov Report

Merging #7006 (15600c3) into main (290280e) will decrease coverage by 1.76%.
The diff coverage is 56.96%.

❗ Current head 15600c3 differs from pull request most recent head be55c28. Consider uploading reports for the commit be55c28 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7006      +/-   ##
==========================================
- Coverage   70.48%   68.71%   -1.77%     
==========================================
  Files         515      554      +39     
  Lines       23150    25698    +2548     
==========================================
+ Hits        16317    17659    +1342     
- Misses       5776     6834    +1058     
- Partials     1057     1205     +148     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 52.00% <ø> (-1.85%) ⬇️
cmd/skaffold/app/cmd/dev.go 84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go 36.66% <0.00%> (-4.72%) ⬇️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/lsp.go 28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/fix.go 68.85% <40.00%> (-7.62%) ⬇️
cmd/skaffold/app/cmd/lint.go 42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.88% <50.00%> (+0.24%) ⬆️
cmd/skaffold/app/skaffold.go 76.19% <70.00%> (-8.43%) ⬇️
... and 177 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 8cd8b69...be55c28. Read the comment docs.

Copy link
Contributor

@gsquared94 gsquared94 left a comment

Choose a reason for hiding this comment

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

lgtm, except one typo and one good to have testcase.

pkg/skaffold/tag/git_commit.go Show resolved Hide resolved
pkg/skaffold/tag/git_commit_test.go Outdated Show resolved Hide resolved
pkg/skaffold/tag/git_commit_test.go Show resolved Hide resolved
@gsquared94 gsquared94 added the kokoro:force-run forces a kokoro re-run on a PR label Jan 13, 2022
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Jan 13, 2022
@snickell
Copy link
Contributor Author

I'm trying to figure out why the linter check is breaking here:

Error: pkg/skaffold/tag/git_commit_test.go:761:48: (*gitRepo).branch - result 0 (*github.com/GoogleContainerTools/skaffold/pkg/skaffold/tag.gitRepo) is never used (unparam)
make: *** [Makefile:126: linters] Error 1
func (g *gitRepo) branch(newBranchName string) *gitRepo {

Unfortunately this is my first go PR, I can't quite see how branch() is referencing *gitRepo (or not) any differently than the parallel functions like commit()..... could somebody with more go experience take a look and suggest?

@snickell
Copy link
Contributor Author

Oh I see, its that nothing is (currently) using the *gitRepo typed return value of branch(), because its always the last in our existing tests. I'm adding a no-op .commit('second') after one of the branches to provide a ref and please the linter.

@snickell snickell changed the title Git tag variant branches feat: implement 'branches' git tagger Jan 13, 2022
@snickell snickell changed the title feat: implement 'branches' git tagger feat: implement new git tag policy variant: 'branches' Jan 13, 2022
@snickell snickell changed the title feat: implement new git tag policy variant: 'branches' feat: add 'branches' git tag variant Jan 13, 2022
@snickell
Copy link
Contributor Author

OK, I think this might be ready for merge pending review by @tejal29

@gsquared94 gsquared94 added the kokoro:force-run forces a kokoro re-run on a PR label Jan 14, 2022
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Jan 14, 2022
@gsquared94 gsquared94 merged commit 4e6eb0c into GoogleContainerTools:main Jan 14, 2022
@snickell
Copy link
Contributor Author

@gsquared94 Do the schemas need manual editing to add the branch tag policy, or is that something that's automatically done on the skaffold build system side?

@snickell
Copy link
Contributor Author

Aloha @gsquared94 / @briandealwis , I was wondering if my PR not editing the schemas was a mistake? Or are schemas going to update automatically at some point?

@briandealwis
Copy link
Member

@snickell the schemas and the docs will need to be updated separately.

@snickell
Copy link
Contributor Author

snickell commented Feb 1, 2022

OK, I'll open a separate PR for that, sorry I thought those might be auto-generated by a github action or something, my bad!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tagPolicy: gitCommit: variant: Branches
4 participants