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

refactor(Stream): rename argument names of mergeLeft and mergeRight from self/that to left/right for clarity #3160

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

dilame
Copy link
Contributor

@dilame dilame commented Jul 4, 2024

Type

In the documentation for mergeLeft it is written

Merges this stream and the specified stream together, discarding the values from the right stream.

But the declaration contains self and that arg names, so i need to watch generics to understand what is "left" and what is "right", which is an extra mental overhead. I think it's more user friendly to name arguments according to documentation and function name:)

If it will be merged, i will refactor the rest functions with left/right semantic.

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Related

  • Related Issue #
  • Closes #

@dilame dilame requested a review from mikearnaldi as a code owner July 4, 2024 02:01
Copy link

changeset-bot bot commented Jul 4, 2024

🦋 Changeset detected

Latest commit: 1e85322

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
effect Minor
@effect/cli Major
@effect/cluster-browser Major
@effect/cluster-node Major
@effect/cluster-workflow Major
@effect/cluster Major
@effect/experimental Major
@effect/opentelemetry Major
@effect/platform-browser Major
@effect/platform-bun Major
@effect/platform-node-shared Major
@effect/platform-node Major
@effect/platform Major
@effect/printer-ansi Major
@effect/printer Major
@effect/rpc-http Major
@effect/rpc Major
@effect/schema Major
@effect/sql-d1 Major
@effect/sql-drizzle Major
@effect/sql-mssql Major
@effect/sql-mysql2 Major
@effect/sql-pg Major
@effect/sql-sqlite-bun Major
@effect/sql-sqlite-node Major
@effect/sql-sqlite-react-native Major
@effect/sql-sqlite-wasm Major
@effect/sql Major
@effect/typeclass Major
@effect/vitest Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot changed the base branch from main to next-minor July 4, 2024 02:02
@dilame dilame force-pushed the merge-left-right branch 2 times, most recently from 0a9b20b to ad86900 Compare July 4, 2024 12:55
@mikearnaldi mikearnaldi merged commit 9a9d999 into Effect-TS:next-minor Jul 4, 2024
@github-actions github-actions bot mentioned this pull request Jul 4, 2024
github-actions bot pushed a commit that referenced this pull request Jul 4, 2024
github-actions bot pushed a commit that referenced this pull request Jul 4, 2024
github-actions bot pushed a commit that referenced this pull request Jul 5, 2024
github-actions bot pushed a commit that referenced this pull request Jul 5, 2024
github-actions bot pushed a commit that referenced this pull request Jul 5, 2024
github-actions bot pushed a commit that referenced this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants