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

Merge Handles::replace_with_multiple into Handles::replace #2084

Merged
merged 2 commits into from
Nov 8, 2023
Merged

Conversation

hannobraun
Copy link
Owner

From the message of the main commit:

Having two different versions of the same method is not justified, in my opinion:

  • The convenience of having a single-replacement special version is so minimal, it doesn't justify any additional complexity.
  • The minimal win in convenience is more than made up for by the inconvenience of a longer name for the general version.

Having a single, general replace method is much better.

This is a follow-up to #2083, and also came out of my work on #2023.

Having two different versions of the same method is not justified, in my
opinion:

- The convenience of having a single-replacement special version is so
  minimal, it doesn't justify any additional complexity.
- The minimal win in convenience is more than made up for by the
  inconvenience of a longer name for the general version.

Having a single, general `replace` method is much better.
@hannobraun hannobraun enabled auto-merge November 8, 2023 11:01
@hannobraun hannobraun merged commit bc2ee5d into main Nov 8, 2023
4 checks passed
@hannobraun hannobraun deleted the handles branch November 8, 2023 11:03
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.

1 participant