Skip to content

Make NonEmptyTuple a Product #9035

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

Merged

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented May 25, 2020

Baby step towards #9033

@nicolasstucki nicolasstucki requested a review from smarter May 25, 2020 09:42
@nicolasstucki nicolasstucki self-assigned this May 25, 2020
@@ -0,0 +1,5 @@
val a: Product = 1 *: ()
Copy link
Member

Choose a reason for hiding this comment

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

This is missing testcases that actually call methods defined on Product and verify they work correctly. (Also I don't know anything about how Tuple is supposed to work, so I don't know if this change is good or bad, someone else should review it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the test to tests/run and added some calls to Product methods. Note that in scala.runtime.Tuple we also have plenty of calls to Product on these tuples.

@nicolasstucki nicolasstucki force-pushed the make-NonEmptyTuple-Product branch from 902cef6 to 31b7a65 Compare May 25, 2020 10:16
@nicolasstucki nicolasstucki requested a review from liufengyun May 25, 2020 10:20
@nicolasstucki nicolasstucki marked this pull request as ready for review May 25, 2020 11:01
@nicolasstucki nicolasstucki dismissed smarter’s stale review May 25, 2020 11:02

Test was re-written

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM

@nicolasstucki nicolasstucki merged commit a3e69f1 into scala:master May 25, 2020
@nicolasstucki nicolasstucki deleted the make-NonEmptyTuple-Product branch May 25, 2020 18:46
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.

3 participants