Skip to content

Comments

feat(isolated-declarations): add builder methods to options struct#6373

Closed
DonIsaac wants to merge 1 commit intodon/10-08-feat_isolated-declarations_impl_default_for_optionsfrom
don/10-08-feat_isolated-declarations_add_builder_methods_to_options_struct
Closed

feat(isolated-declarations): add builder methods to options struct#6373
DonIsaac wants to merge 1 commit intodon/10-08-feat_isolated-declarations_impl_default_for_optionsfrom
don/10-08-feat_isolated-declarations_add_builder_methods_to_options_struct

Conversation

@DonIsaac
Copy link
Contributor

@DonIsaac DonIsaac commented Oct 8, 2024 •
edited
Loading

Allows constructing and modifying options in a way that will not break if we add new properties to IsolatedDeclarationsOptions.

// breaks as soon as a new member is added
let opts = IsolatedDeclarationsOptions { strip_internal: true }
// resilient to future contract changes
let opts = IsolatedDeclarationsOptions::default().strip_internal()

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 8, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@DonIsaac Graphite App
Copy link
Contributor Author

DonIsaac commented Oct 8, 2024 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @DonIsaac and the rest of your teammates on Graphite Graphite

@DonIsaac DonIsaac mentioned this pull request Oct 8, 2024
Merged
@github-actions github-actions bot added the A-isolated-declarations Isolated Declarations label Oct 8, 2024
@DonIsaac DonIsaac marked this pull request as ready for review October 8, 2024 17:14
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 8, 2024

CodSpeed Performance Report

Merging #6373 will not alter performance

Comparing don/10-08-feat_isolated-declarations_add_builder_methods_to_options_struct (ae3e9f0) with don/10-08-feat_isolated-declarations_impl_default_for_options (4210c8b)

Summary

✅ 29 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Oct 9, 2024

// breaks as soon as a new member is added
let opts = IsolatedDeclarationsOptions { strip_internal: true }

I'm leaning towards forcing the breaking change.

And also ... I want to ban builder pattern at some point.

@Boshen Boshen closed this Oct 9, 2024
@Boshen Boshen deleted the don/10-08-feat_isolated-declarations_add_builder_methods_to_options_struct branch October 9, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

A-isolated-declarations Isolated Declarations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DonIsaac @Boshen