-
Notifications
You must be signed in to change notification settings - Fork 8
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 LaunchBuilder #487
Add LaunchBuilder #487
Conversation
Currently looking into a fix for the integration test failures that are triggered by us having multiple Edit: Solved via 0e4279c |
89cf784
to
89cc7f8
Compare
89cc7f8
to
0e4279c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
A little late on the draw, but I reviewed this. Looks good. I see that the Do we have examples of building a slice or should we just manually create the struct for now libcnb.rs/libcnb-data/src/launch.rs Lines 225 to 232 in d0a35f7
|
This PR originally started as a fix for #485. As it turns out, adding slices to
Launch
was already possible, albeit more complicated as it should be since there were no builder functions available for slices.This PR adds a dedicated
LaunchBuilder
that allows setting all fields forLaunch
, removes the builder-style functions fromLaunch
as they were inconsistent with the rest of libcnb.rs anyways. I still want to tackle #166, but this isn't the PR for that.I also added some docs for
Slice
that clarify whatpaths
actually is (and renamed it). I opted to not implement a newtype for Go standard library path globs for now as we can spend our time better in other areas. I created an issue for it as #486.Changelog
Launch
with a dedicatedLaunchBuilder
to be more consistent with other builders in the library. Additionally, all fields ofLaunch
can now be modified via the builder pattern.paths
field inlaunch::Slice
topath_globs
and add docs to make it clearer that these strings are Go standard library globs.Closes #485, GUS-W-11486542