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

Add option to configure default platforms #897

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

ReToCode
Copy link
Contributor

@ReToCode ReToCode commented Dec 1, 2022

Adds an option to configure default platforms either in .ko.yaml or using the env variable KO_DEFAULTPLATFORMS.

Fixes #521

@ReToCode
Copy link
Contributor Author

ReToCode commented Dec 1, 2022

@imjasonh something like this? Unfortunately there is no easy way to test this properly without either

  • exposing internals of gobuildOpener to test this from resolver_test.go
  • or extending the build.Interface interface similar to QualifyImport to test it in resolver_test.go

WDYT?

@imjasonh
Copy link
Member

imjasonh commented Dec 2, 2022

This looks great! This is honestly even less code than I thought it would be. 👍

The test seems fine. Maybe worth adding another .ko.yaml that has defaultPlatforms: foo,bar just to make sure it handles splitting on , in the config as we expect.

@ReToCode ReToCode force-pushed the feature/default-platforms branch 2 times, most recently from fa1c902 to c486001 Compare December 2, 2022 15:40
@ReToCode
Copy link
Contributor Author

ReToCode commented Dec 2, 2022

I added another test case but not with splitting , as viper expects a yaml array to be parsed as a string slice. I also changed the docs accordingly.

imjasonh
imjasonh previously approved these changes Dec 2, 2022
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.

Nice, that test covers exactly what I was hoping to find out!

jonjohnsonjr
jonjohnsonjr previously approved these changes Dec 2, 2022
@ReToCode ReToCode dismissed stale reviews from jonjohnsonjr and imjasonh via b4371bc December 5, 2022 06:29
imjasonh
imjasonh previously approved these changes Dec 6, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #897 (b4371bc) into main (a28ed35) will increase coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
+ Coverage   51.33%   51.34%   +0.01%     
==========================================
  Files          44       44              
  Lines        3407     3414       +7     
==========================================
+ Hits         1749     1753       +4     
- Misses       1430     1432       +2     
- Partials      228      229       +1     
Impacted Files Coverage Δ
pkg/commands/resolver.go 36.55% <0.00%> (-0.34%) ⬇️
pkg/commands/options/build.go 71.07% <100.00%> (+0.98%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ReToCode
Copy link
Contributor Author

ReToCode commented Dec 7, 2022

Seems like there was already an empty line in that file which is not really visible in GitHub.

@ReToCode ReToCode force-pushed the feature/default-platforms branch from 2b46068 to ab8e3c1 Compare December 11, 2022 08:32
@imjasonh imjasonh merged commit 7dc51c0 into ko-build:main Dec 12, 2022
@imjasonh
Copy link
Member

Thanks again for this contribution, and for fighting through the lint checks for it! 😅

@ReToCode ReToCode deleted the feature/default-platforms branch December 12, 2022 15:52
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.

FR: configure platforms in .ko.yaml
4 participants