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

Support builder description in builder.toml #176

Merged
merged 1 commit into from
May 9, 2019

Conversation

ameyer-pivotal
Copy link
Contributor

  • Shown in inspect-builder output
  • Shown in suggested builder output (use default descriptions if not present)

[#172]

Signed-off-by: Andrew Meyer [email protected]

- Shown in inspect-builder output
- Shown in suggested builder output (use default descriptions if not present)

[#172]

Signed-off-by: Andrew Meyer <[email protected]>
@@ -194,6 +198,10 @@ func hasBPWithVersion(bps []BuildpackMetadata, version string) bool {
return false
}

func (b *Builder) SetDescription(description string) {
Copy link
Member

Choose a reason for hiding this comment

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

This is starting to look like Java. Why do we need getters/setters here? I see we're returning a concrete *Builder in the constructor. If we need to change these properties via an interface, could we refactor to pull the state out into data structs?

Copy link
Contributor Author

@ameyer-pivotal ameyer-pivotal May 9, 2019

Choose a reason for hiding this comment

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

@sclevine This type follows a builder-esque pattern (not to be confused with a CNB builder lol) so is stateful. If you still think it should be changed, I'm not sure I'm following your proposed solution. Could you clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per side conversation, I'll capture this in a chore and merge as is

@ameyer-pivotal ameyer-pivotal dismissed sclevine’s stale review May 9, 2019 21:36

Capturing in chore

@ameyer-pivotal ameyer-pivotal merged commit afed298 into master May 9, 2019
@ekcasey ekcasey deleted the builder-description branch May 14, 2019 21:46
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.

2 participants