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 Buildpack environment related content #754

Closed
wants to merge 14 commits into from
Closed

Add Buildpack environment related content #754

wants to merge 14 commits into from

Conversation

hyounes4560
Copy link
Contributor

@hyounes4560 hyounes4560 commented Jul 19, 2024 •
edited
Loading

@hyounes4560
adds page summaries and fixes few MD styling
d359717 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
@hyounes4560 hyounes4560 requested a review from a team as a code owner July 19, 2024 23:58
@hyounes4560
adds more details about env variables
6939904 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
AidanDelaney
AidanDelaney requested changes Jul 22, 2024
View reviewed changes
Copy link
Member

@AidanDelaney AidanDelaney left a comment

Choose a reason for hiding this comment

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

This looks really good. Just one comment.

hyounes4560 and others added 3 commits August 6, 2024 22:49
@hyounes4560
adds reviewer feedback - 1
4d6a4fa 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
@hyounes4560
Merge branch 'main' into expand-env-pages
598b805
@hyounes4560
Implements reviewers feedback
6083533 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
natalieparellano
natalieparellano reviewed Aug 14, 2024
View reviewed changes
content/docs/for-buildpack-authors/how-to/write-buildpacks/clear-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/clear-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/clear-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/clear-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/clear-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
@hyounes4560
adds latest reviewer feedback
4efbd42 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
@hyounes4560
reorganizes a section
29de803 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
natalieparellano
natalieparellano reviewed Aug 20, 2024
View reviewed changes
content/docs/for-buildpack-authors/how-to/write-buildpacks/clear-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/clear-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md
* `User-defined` variables are then applied; meaning it's possible to override `buildpack-defined` variables.
* Finally, `platform-defined` variables are applied, which eventually override any previous values.

### Preparing the environment at runtime
Copy link
Member

Choose a reason for hiding this comment

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

I think we can flesh out this section a little more

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could you elaborate on this? thanks

Copy link
Member

Choose a reason for hiding this comment

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

I'm happy to land this change now, and flesh out this discussion in subsequent changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @AidanDelaney

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@natalieparellano still waiting for you

content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
content/docs/for-buildpack-authors/how-to/write-buildpacks/specify-env.md Outdated Show resolved Hide resolved
AidanDelaney
AidanDelaney reviewed Aug 20, 2024
View reviewed changes
Copy link
Member

@AidanDelaney AidanDelaney left a comment

Choose a reason for hiding this comment

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

I'm happy with this and @natalieparellano's suggested changes.

natalieparellano and others added 2 commits August 21, 2024 10:05
@natalieparellano
Merge branch 'main' into expand-env-pages
ab0b470
@hyounes4560
adds more reviewers feedback
ad029b0 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
AidanDelaney
AidanDelaney approved these changes Aug 23, 2024
View reviewed changes
Copy link
Member

@AidanDelaney AidanDelaney left a comment

Choose a reason for hiding this comment

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

LGTM :)

@hyounes4560
adds final reviewer feedback
5c1d9e1
@hyounes4560
adds minor edit
c1f8c5e 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
@hyounes4560
fixes a typo
a7759e8 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
@hyounes4560
fixes a typo
6fa94e8 
Signed-off-by: Hanan Younes <hanan.algazaley@gmail.com>
@hyounes4560
Merge branch 'expand-env-pages' of github.com:hyounes4560/docs into e…
c40a7e5 
…xpand-env-pages
@AidanDelaney
Copy link
Member

In order to fix the DCO check I created #764, so I'll close this PR in favour of that one.

Normally I'd git rebase -i --signoff original-branch-point-sha, which ensures that all commits are signed off. In this case there are multiple merges of main onto this branch, so I instead squashed a bunch of commits into a single commit, with signoff.

If we land #764, then we can close this PR too :)

@AidanDelaney
Copy link
Member

Now merged as #764

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natalieparellano natalieparellano natalieparellano left review comments

@AidanDelaney AidanDelaney AidanDelaney approved these changes

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

Successfully merging this pull request may close these issues.

Expand the 2 env variable related pages
3 participants
@hyounes4560 @AidanDelaney @natalieparellano