Skip to content

Accept .5 for half #6746

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
merged 2 commits into from
Jun 8, 2018
Merged

Accept .5 for half #6746

merged 2 commits into from
Jun 8, 2018

Conversation

som-snytt
Copy link
Contributor

Previous refactor pruned too much by half.

@scala-jenkins scala-jenkins added this to the 2.13.0-M5 milestone Jun 6, 2018
@lrytz lrytz requested a review from martijnhoekstra June 7, 2018 07:25
@@ -242,6 +242,7 @@ class StringParsersTest {
"-0x0.000000000000090000000000000000001p-1022",
"-0x0.00000000000009fffffffffffffffffffffffffffffffffp-1022",
"-0x0.0000000000000fffffffffffffffffffffffffffffffffffp-1022",
".4",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get "", "." and "4." too while we're at 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.

OK, I was too busy with "half" puns.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe change to "0.5" then 👍

Previous refactor pruned too much by half.
@SethTisue
Copy link
Member

reduce to two commits for merge...?

As requested, more dotted tests.
No benchmarks around the refactor.
@som-snytt
Copy link
Contributor Author

I squashed the new commits to .5 or half the total.

Copy link
Member

@SethTisue SethTisue left a comment

Choose a reason for hiding this comment

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

assuming CI likes it.

to anyone confused about what the context for this, it's #6726

@som-snytt
Copy link
Contributor Author

The context is my hasty review of same. I was all ferocious about boolean expressions, irrespective of truth value.

@SethTisue SethTisue merged commit 4d6a9ba into scala:2.13.x Jun 8, 2018
@som-snytt som-snytt deleted the review/6726-more branch June 8, 2018 04:38
@som-snytt
Copy link
Contributor Author

Woo-hoo, off to rebase all my outstanding PRs! Outstanding in both senses.

@SethTisue
Copy link
Member

out standing in the rain.

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.

4 participants