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

warning users when using both --base-import-paths --bare flags #618

Merged
merged 2 commits into from
Feb 28, 2022
Merged

warning users when using both --base-import-paths --bare flags #618

merged 2 commits into from
Feb 28, 2022

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Feb 25, 2022

warning users when using both --base-import-paths --bare flags

Partially fixes #542

@imjasonh let me know if this is something you have in mind, otherwise, if this is not the project want we can close or improve. thanks!

$ ./main build --base-import-paths  --bare .
2022/02/25 17:55:08 WARNING!
-----------------------------------------------------------------
Both --base-import-paths and --bare was set.

--base-import-paths will take precedence and ignore --bare flag.

In future this will be an error
-----------------------------------------------------------------
2022/02/25 17:55:10 Using base golang:1.17@sha256:e06c83493ef6d69c95018da90f2887bf337470db074d3c648b8b648d8e3c441e for github.com/google/ko
2022/02/25 17:55:10 Building github.com/google/ko for linux/amd64
^C2022/02/25 17:55:11 Unexpected error running "go build": signal: killed
Error: failed to publish images: error building "ko://github.com/google/ko": signal: killed
2022/02/25 17:55:11 error during command execution:failed to publish images: error building "ko://github.com/google/ko": signal: killed

Copy link
Member

@imjasonh imjasonh left a comment

Choose a reason for hiding this comment

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

Looks great to me, thanks for doing this! Just a few tiny style/naming things, but otherwise 👍

pkg/commands/apply.go Outdated Show resolved Hide resolved
pkg/commands/apply.go Outdated Show resolved Hide resolved
pkg/commands/options/validate.go Outdated Show resolved Hide resolved
pkg/commands/options/validate.go Outdated Show resolved Hide resolved
Signed-off-by: Carlos Panato <ctadeu@gmail.com>
@cpanato
Copy link
Member Author

cpanato commented Feb 25, 2022

thanks @imjasonh for the quick review and feedback, I've addressed your comments PTAL when have some free cycle

imjasonh
imjasonh previously approved these changes Feb 25, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 25, 2022

Codecov Report

Merging #618 (e9ce2d3) into main (ea93812) will decrease coverage by 0.31%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #618      +/-   ##
==========================================
- Coverage   50.72%   50.40%   -0.32%     
==========================================
  Files          43       44       +1     
  Lines        3316     3337      +21     
==========================================
  Hits         1682     1682              
- Misses       1424     1445      +21     
  Partials      210      210              
Impacted Files Coverage Δ
pkg/commands/apply.go 40.36% <0.00%> (-1.15%) ⬇️
pkg/commands/build.go 61.29% <0.00%> (-3.12%) ⬇️
pkg/commands/create.go 37.60% <0.00%> (-0.99%) ⬇️
pkg/commands/options/validate.go 0.00% <0.00%> (ø)
pkg/commands/resolve.go 67.92% <0.00%> (-4.08%) ⬇️
pkg/commands/run.go 21.23% <0.00%> (-0.58%) ⬇️

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 ea93812...e9ce2d3. Read the comment docs.

Signed-off-by: cpanato <ctadeu@gmail.com>
@cpanato
Copy link
Member Author

cpanato commented Feb 28, 2022

@imjasonh fixed the header ready to go

@imjasonh imjasonh merged commit 62800bc into ko-build:main Feb 28, 2022
@cpanato cpanato deleted the GH-542 branch February 28, 2022 15:15
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.

Improve errors/warnings for unnecessary .ko.yaml settings
3 participants