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.

Sign up for GitHub

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

Jump to bottom

Suggested changes to s2i_strategy #1

Merged
merged 8 commits into from
Aug 25, 2020
Merged

Suggested changes to s2i_strategy #1

merged 8 commits into from
Aug 25, 2020

Conversation

wtam2018
Copy link
Collaborator

  • Got rid of BuilderImageKind
  • Renamed BuildImage to BuildImageTag
  • Add "name" for SourceToImage in schema
  • Fix comments
  • Refactored CreateSourceBuildConfigWithBinaryInput and CreateBuildConfig()
  • Refactored generateSourceBuildConfigWithBinaryInput() and generateBuildConfig()

What type of PR is this?

Uncomment only one /kind line, and delete the rest.
For example, > /kind bug would simply become: /kind bug

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind failing-test
/kind feature
/kind flake
/kind code-refactoring

Documentation changes: Please include [skip ci] in your commit message as well
/kind documentation
[skip ci]

What does does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #?

How to test changes / Special notes to the reviewer:

@wtam2018
* Got rid of BuilderImageKind
a2ac697 
* Renamed BuildImage to BuildImageTag
* Add "name" for SourceToImage in schema
* Fix  comments
* Refactored CreateSourceBuildConfigWithBinaryInput and CreateBuildConfig()
* Refactored generateSourceBuildConfigWithBinaryInput() and generateBuildConfig()
@Shraddhak22
Copy link
Owner

https://github.com/Shraddhak22/odo/pull/1/commits/a2ac6973046f5acb83c75089b9d4ce5803a226e9#diff-13da4255a94b68b3d97da843ec78051aR1213
Can we keep BuilderImage only instead of BuilderImageTag. BuilderImageTag sounds confusing, or we can have BuilderImagewithTag(But then it goes really long)

@wtam2018
Copy link
Collaborator Author

https://github.com/Shraddhak22/odo/pull/1/commits/a2ac6973046f5acb83c75089b9d4ce5803a226e9#diff-13da4255a94b68b3d97da843ec78051aR1213
Can we keep BuilderImage only instead of BuilderImageTag. BuilderImageTag sounds confusing, or we can have BuilderImagewithTag(But then it goes really long)

I am fine with BuilderImage but ImageTag is widely used as the terminology in the existing code that handles builder image in occlient and the Kind is assumed to be ImageStreamTag.

Shraddhak22
Shraddhak22 reviewed Aug 24, 2020
View reviewed changes
pkg/devfile/parser/data/2.1.0/devfileJsonSchema210.go Outdated
"builderImage":{
"description":"Mandatory builder image name with tag",
"builderImageTag":{
"description":"Mandatory name builder image name with tag",
Copy link
Owner

Choose a reason for hiding this comment

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

"name" coming twice in description for builderImageTag

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

@wtam2018
*Fix coments/Description
8263eee 
* Refactor EnsureImageStream() to make sure output image stream is present for BinaryInput Builds.
@wtam2018
* Get rid of EnsureImageStream. It turns out we don't need to make su…
46f898f 
…re output imagestream exist before calling CreateBuildConfig()

* Pass output image kind to generateBuildConfig.
@wtam2018
* revert to do s2i build and add comment.
3c9cd64
@wtam2018
Rename builderImageTag to builderImageStreamTag
70d42ef
@wtam2018
relax validation check to allow empty tag (image repository) which d…
7724c79 
…efault to the "buildName"
@wtam2018
Call EnsureImageStream if --tag flag is not provided
79b9798
@wtam2018
* Add enum for BuildGuildanceType
f1256c7 
* Use build guidance structs in  CLI parameters
* Switch build strategy based on guidance in devfiile.    We will use the first build guidance we find.
* Build guidance is not a component in the schema.   It has its own type "BuildGuidances"
@wtam2018 wtam2018 requested a review from Shraddhak22 August 25, 2020 11:52
@wtam2018 wtam2018 merged commit 290efb5 into Shraddhak22:s2i_strategy Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shraddhak22 Shraddhak22 Awaiting requested review from Shraddhak22

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wtam2018 @Shraddhak22