Skip to content

chore: rename innerJoin to intersectionWith#3225

Open
adispring wants to merge 2 commits into
ramda:masterfrom
adispring:chore/rename-innerJoin-to-intersectionWith
Open

chore: rename innerJoin to intersectionWith#3225
adispring wants to merge 2 commits into
ramda:masterfrom
adispring:chore/rename-innerJoin-to-intersectionWith

Conversation

@adispring
Copy link
Copy Markdown
Member

@adispring adispring commented Jan 31, 2022

innerJoin is a sql command, R.innerJoin only has inner but no join, so rename it to intersectionWith may be more accurate?

customcommander
customcommander reviewed Jan 31, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@customcommander customcommander left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Of course that's a breaking change ;) Question: did you rename the test file?

@adispring
Copy link
Copy Markdown
Member Author

adispring commented Jan 31, 2022

@customcommander , Yes, thanks for remind.

customcommander
customcommander approved these changes Jan 31, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@customcommander customcommander left a comment

Choose a reason for hiding this comment

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

LGTM and makes perfect sense. Would like another approval just in case. Otherwise let's merge this end of week latest?

@customcommander
Copy link
Copy Markdown
Member

customcommander commented Feb 4, 2022

@CrossEye Any objection?

@CrossEye
Copy link
Copy Markdown
Member

CrossEye commented Feb 5, 2022

I agree with the change.

I guess ,,we need to decide whether 1.0 or 0.29 is next. When we rename a function, we usually keep the old name around as an alias for a minor version. If we're building 0.29 I would do so, but I think if we're headed right for 1.0, then we need to skip it this time.

@customcommander
Copy link
Copy Markdown
Member

customcommander commented Feb 6, 2022

@CrossEye: I wouldn't bother with 0.29 at this point and would rather keep the momentum for 1.0.0 and ship as many breaking changes in that one.

@Retro64 Retro64 mentioned this pull request May 16, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@customcommander customcommander customcommander approved these changes

@CrossEye CrossEye Awaiting requested review from CrossEye

@hitmands hitmands Awaiting requested review from hitmands

@Bradcomp Bradcomp Awaiting requested review from Bradcomp

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adispring @customcommander @CrossEye