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

Set statuscheck to false. #2499

Merged
merged 3 commits into from
Jul 18, 2019
Merged

Conversation

tejal29
Copy link
Contributor

@tejal29 tejal29 commented Jul 18, 2019

Turn status-check false by default as a workaround for #2496 and #2495.

@@ -66,7 +66,7 @@ var FlagRegistry = []Flag{
Shorthand: "n",
Usage: "Run deployments in the specified namespace",
Value: &opts.Namespace,
DefValue: "",
DefValue: "default",
Copy link
Contributor

Choose a reason for hiding this comment

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

let's separate this out to a different PR with tests and everything, I'm not sure about that this is what we want to do (we might but I need a bit more time to think it through).

Copy link
Contributor

Choose a reason for hiding this comment

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

more thought around this: there is a difference between leaving it empty and letting the resources determine their namespace vs enforcing default.

@tejal29 tejal29 changed the title Default namespace to "default" and turn status check to false. Set statuscheck to false. Jul 18, 2019
Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Jul 18, 2019

Codecov Report

Merging #2499 into master will not change coverage.
The diff coverage is n/a.

Impacted Files Coverage Δ
cmd/skaffold/app/cmd/flags.go 100% <ø> (ø) ⬆️

@balopat balopat merged commit cc673b5 into GoogleContainerTools:master Jul 18, 2019
@tejal29 tejal29 deleted the fix_2495 branch April 15, 2021 07:33
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.

3 participants