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

Skaffold master -> main #6263

Merged
merged 3 commits into from
Jul 21, 2021

Conversation

briandealwis
Copy link
Member

Update references to master to now point to main.

Rewrite the few existing uses of `skaffold/(blob|tree)/HEAD` to use main
as GitHub's file viewer uses a commit rather than a branch.

Rewrite the buildpacks to use main since they already switched.
@briandealwis briandealwis requested a review from a team as a code owner July 21, 2021 16:48
@briandealwis briandealwis requested a review from tejal29 July 21, 2021 16:48
@google-cla google-cla bot added the cla: yes label Jul 21, 2021
@tejal29
Copy link
Contributor

tejal29 commented Jul 21, 2021

Verification notes

hub pr checkout 6263

rm -rf docs/static/swaggerr

git grep master

Binary file hack/release-notes matches

@briandealwis briandealwis enabled auto-merge (squash) July 21, 2021 16:58
Copy link
Contributor

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Can you verify if hack/release_notes binary needs to be rebuilt.
See #6263 (comment)

@briandealwis
Copy link
Member Author

briandealwis commented Jul 21, 2021

hack/release-notes is a binary executable and shouldn't have been committed! 😱

@tejal29
Copy link
Contributor

tejal29 commented Jul 21, 2021

hack/release-notes is a binary executable and shouldn't be committed! 😱

Maybe it was mistake during v1.9.0?
https://github.com/GoogleContainerTools/skaffold/blob/main/hack/release-notes

@briandealwis
Copy link
Member Author

I don't find any references to master in hack/tools/vendor/github.com/corneliusweig/release-notes.

@codecov
Copy link

codecov bot commented Jul 21, 2021

Codecov Report

Merging #6263 (a1086fd) into main (8c34f1e) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6263      +/-   ##
==========================================
+ Coverage   70.86%   70.88%   +0.01%     
==========================================
  Files         490      490              
  Lines       22162    22162              
==========================================
+ Hits        15706    15709       +3     
+ Misses       5440     5439       -1     
+ Partials     1016     1014       -2     
Impacted Files Coverage Δ
pkg/skaffold/build/docker/docker.go 89.28% <ø> (ø)
pkg/skaffold/docker/image.go 69.93% <0.00%> (+1.04%) ⬆️

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 8c34f1e...a1086fd. Read the comment docs.

Copy link
Contributor

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Please change examples/remote-multi-config-microservices to include branch: main

- git:
    repo: https://github.com/GoogleContainerTools/skaffold
    path: examples/multi-config-microservices/leeroy-app
    sync: false
- git:
    repo: https://github.com/GoogleContainerTools/skaffold
    path: examples/multi-config-microservices/leeroy-web
    sync: false
``

@briandealwis
Copy link
Member Author

briandealwis commented Jul 21, 2021

We don't need to do that @tejal29: the git repo code already uses main if master is not found.

// defaultRef returns the default ref as "master" if master branch exists in
// remote repository, falls back to "main" if master branch doesn't exist
func defaultRef(repo string) (string, error) {

Well at least theoretically that's supposed to work :-) (#6264)

--- FAIL: TestRun (326.07s)
/integration/TestRun/remote-multi-config-microservices
time="2021-07-21T17:12:58Z" level=info msg="Namespace: skaffold6b8pk"
time="2021-07-21T17:12:58Z" level=info msg="Running [skaffold run --namespace skaffold6b8pk --default-repo gcr.io/k8s-skaffold --cache-artifacts=false] in examples/remote-multi-config-microservices"
    helper.go:239: skaffold run: exit status 128, 
    panic.go:613: parsing skaffold config: error parsing skaffold configuration file: caching remote dependency https://github.com/GoogleContainerTools/skaffold: failed to clone repo: running [/usr/bin/git clone https://github.com/GoogleContainerTools/skaffold euw2xNJipHV7cZ9CraIE8t5SYEhMAt9x --branch master --depth 1]
         - stdout: ""
         - stderr: "Cloning into 'euw2xNJipHV7cZ9CraIE8t5SYEhMAt9x'...\nwarning: Could not find remote branch master to clone.\nfatal: Remote branch master not found in upstream origin\n"
         - cause: exit status 128
    --- FAIL: TestRun/remote-multi-config-microservices (0.71s)

Copy link
Contributor

@aaron-prindle aaron-prindle left a comment

Choose a reason for hiding this comment

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

Do we have any build/release webhooks (eg: cloudbuild, etc) which target master that need to be changed on GCP (code/infra not directly in this repo)? I don't believe so, just wanted to put this here in case something was there that watched master

@briandealwis
Copy link
Member Author

@aaron-prindle I updated the GCB triggers already to support master|main
https://console.cloud.google.com/cloud-build/triggers?project=k8s-skaffold

@briandealwis briandealwis added the kokoro:run runs the kokoro jobs on a PR label Jul 21, 2021
@briandealwis briandealwis disabled auto-merge July 21, 2021 20:27
@briandealwis briandealwis added the kokoro:force-run forces a kokoro re-run on a PR label Jul 21, 2021
@briandealwis
Copy link
Member Author

Kokoro is being temperamental. Committing since everything is green.

@briandealwis briandealwis merged commit 01a8336 into GoogleContainerTools:main Jul 21, 2021
@briandealwis briandealwis deleted the master2main branch July 21, 2021 20:35
yuwenma pushed a commit to yuwenma/skaffold that referenced this pull request Aug 3, 2021
- Rewrite the few existing uses of `skaffold/(blob|tree)/HEAD` to use `main`
  as GitHub's file viewer uses a commit rather than a branch.
- Rewrite the buildpacks references to use `main` since they already switched.
- Explicitly specify `main` for remote repositories due to GoogleContainerTools#6264
yuwenma pushed a commit to yuwenma/skaffold that referenced this pull request Aug 4, 2021
- Rewrite the few existing uses of `skaffold/(blob|tree)/HEAD` to use `main`
  as GitHub's file viewer uses a commit rather than a branch.
- Rewrite the buildpacks references to use `main` since they already switched.
- Explicitly specify `main` for remote repositories due to GoogleContainerTools#6264
tejal29 pushed a commit that referenced this pull request Aug 5, 2021
* [v2] Customize hydration-dir and render-output path.

1. Remove unused flag v2
2. Enable hydration-dir as a configurable flag.
3. Convert structured (hydrated) manifests to a flattened format and stored in a give path (--render-output/--output flag)
4. Prompt user interactive message to acknowledge users that the manifest hydration may override the hydration-dir
5. sort SkaffoldOptions to fix maligned linter error.

* Skaffold master -> main (#6263)

- Rewrite the few existing uses of `skaffold/(blob|tree)/HEAD` to use `main`
  as GitHub's file viewer uses a commit rather than a branch.
- Rewrite the buildpacks references to use `main` since they already switched.
- Explicitly specify `main` for remote repositories due to #6264

* bump v1 versions to v2beta20

* improve user experience on the prompting message

Co-authored-by: Brian de Alwis <[email protected]>
yuwenma pushed a commit to yuwenma/skaffold that referenced this pull request Aug 6, 2021
- Rewrite the few existing uses of `skaffold/(blob|tree)/HEAD` to use `main`
  as GitHub's file viewer uses a commit rather than a branch.
- Rewrite the buildpacks references to use `main` since they already switched.
- Explicitly specify `main` for remote repositories due to GoogleContainerTools#6264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes kokoro:force-run forces a kokoro re-run on a PR kokoro:run runs the kokoro jobs on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants