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

Add Builder extension spec #193

Merged
merged 8 commits into from
Jul 21, 2021
Merged

Add Builder extension spec #193

merged 8 commits into from
Jul 21, 2021

Conversation

dfreilich
Copy link
Member

This adds a Builder specification as an extension spec. It was formalized and agreed upon in buildpacks/rfcs#116.

Resolves #101

@dfreilich
Add initial builder spec
190f2cb 
Signed-off-by: David Freilich <[email protected]>
@dfreilich dfreilich requested a review from a team as a code owner February 15, 2021 13:26
dfreilich
dfreilich commented Feb 15, 2021
View reviewed changes
extensions/builder.md Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
ekcasey
ekcasey requested changes Feb 17, 2021
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
platform.md Outdated Show resolved Hide resolved
extensions/builder.md Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
@dfreilich
Update for comments
557b34f 
Signed-off-by: David Freilich <[email protected]>
@dfreilich
Capitalize ID
7c048c8 
Signed-off-by: David Freilich <[email protected]>
@sclevine sclevine requested a review from a team February 24, 2021 19:08
@sclevine sclevine added this to the Builder 0.1 milestone Feb 24, 2021
jkutner
jkutner reviewed Mar 10, 2021
View reviewed changes
distribution.md Outdated Show resolved Hide resolved
@dfreilich dfreilich mentioned this pull request Mar 10, 2021
Open
2 tasks
@nebhale nebhale requested a review from a team March 17, 2021 18:06
@nebhale nebhale requested a review from ekcasey March 24, 2021 18:30
natalieparellano
natalieparellano reviewed Apr 8, 2021
View reviewed changes
distribution.md Outdated Show resolved Hide resolved
@dfreilich
Incorporate suggestions
dff4ee9 
Signed-off-by: David Freilich <[email protected]>
natalieparellano
natalieparellano reviewed Apr 9, 2021
View reviewed changes
Copy link
Member

@natalieparellano natalieparellano left a comment

Choose a reason for hiding this comment

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

I don't think the other ones are configurable

extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
extensions/builder.md Outdated Show resolved Hide resolved
@hone
Copy link
Member

hone commented Apr 14, 2021

Besides @natalieparellano suggestions, is there anything else blocking this?

@natalieparellano
Copy link
Member

Besides @natalieparellano suggestions, is there anything else blocking this?

If we mark #193 (comment) as out of scope, I don't think so...

dfreilich and others added 2 commits April 14, 2021 19:51
@dfreilich @natalieparellano
Apply suggestions from code review
35b8950 
Signed-off-by: David Freilich <[email protected]>

Co-authored-by: Natalie Arellano <[email protected]>
@dfreilich @natalieparellano
Apply suggestions from code review
85542a2 
Signed-off-by: David Freilich <[email protected]>

Co-authored-by: Natalie Arellano <[email protected]>
@sclevine sclevine requested review from nebhale, ekcasey and sclevine May 12, 2021 18:16
ekcasey
ekcasey approved these changes May 19, 2021
View reviewed changes
extensions/builder.md
- [`io.buildpacks.lifecycle.apis` (JSON)](#iobuildpackslifecycleapis-json)

## General Requirements
A **builder** is an image that MUST contain an implementation of the lifecycle, and build-time environment, and MAY contain buildpacks. Platforms SHOULD use builders to ease the build process.
Copy link
Member

Choose a reason for hiding this comment

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

The wording here still feels a bit awkward to me. However it also seems like a repetition of the first section. Maybe instead of word smithing we should just remove it?

Copy link
Member

Choose a reason for hiding this comment

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

Do you dislike the first sentence?

I do think the second sentence the "SHOULD" probably is a "MAY".

Copy link
Member

Choose a reason for hiding this comment

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

Made a suggestion

extensions/builder.md Show resolved Hide resolved
extensions/builder.md Show resolved Hide resolved
extensions/builder.md Show resolved Hide resolved
extensions/builder.md Show resolved Hide resolved
nebhale
nebhale approved these changes Jun 2, 2021
View reviewed changes
Copy link
Contributor

@nebhale nebhale left a comment

Choose a reason for hiding this comment

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

This will need a new branch rather than a merge back to main. I'll take care of it when the time comes.

@natalieparellano
Copy link
Member

Suggested some nits. #193 (comment) and #193 (comment) seem worthy of resolving before we merge.

@dfreilich
Add default to env var table
cabe24f 
Signed-off-by: David Freilich <[email protected]>
@dfreilich dfreilich force-pushed the 101-builders-extension-spec branch from 2cf0fa4 to cabe24f Compare June 22, 2021 18:43
@hone
Copy link
Member

hone commented Jun 23, 2021

@ekcasey will create a extensions/builder branch to merge this into.

@hone hone assigned ekcasey and unassigned natalieparellano Jun 23, 2021
@ekcasey ekcasey changed the base branch from main to extensions/builder/0.1 July 14, 2021 14:05
@ekcasey
Copy link
Member

ekcasey commented Jul 14, 2021

repointed to extensions/builder/0.1

@ekcasey ekcasey merged commit 4d29e1e into buildpacks:extensions/builder/0.1 Jul 21, 2021
@ekcasey ekcasey mentioned this pull request Jul 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekcasey ekcasey ekcasey approved these changes

@hone hone hone approved these changes

@natalieparellano natalieparellano natalieparellano approved these changes

@sclevine sclevine sclevine approved these changes

@jabrown85 jabrown85 jabrown85 approved these changes

@nebhale nebhale nebhale approved these changes

@jkutner jkutner jkutner approved these changes

Assignees

@ekcasey ekcasey

Labels
api/extensions/builder
Projects
None yet
Milestone
Builder 0.1 (archived)
Development

Successfully merging this pull request may close these issues.

[RFC 0065] Add an extension spec for builders
8 participants
@dfreilich @hone @natalieparellano @ekcasey @jkutner @nebhale @jabrown85 @sclevine