Skip to content
This repository has been archived by the owner on Nov 5, 2022. It is now read-only.

Upgrades and consistent formatting #33

Merged
merged 3 commits into from
May 10, 2018
Merged

Upgrades and consistent formatting #33

merged 3 commits into from
May 10, 2018

Conversation

fommil
Copy link
Contributor

@fommil fommil commented May 10, 2018

No description provided.

@fommil fommil requested review from jmcardon and hrhino May 10, 2018 10:49
@fommil
Copy link
Contributor Author

fommil commented May 10, 2018

@ktonga

hrhino
hrhino approved these changes May 10, 2018
View reviewed changes
Copy link
Member

@hrhino hrhino left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM once you get Travis happy.

(Sundry test sources and build.sbt are ill-fmted)

project/scalafmt.conf
@@ -1,3 +1,4 @@
maxColumn = 120
Copy link
Member

Choose a reason for hiding this comment

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

.............

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't get me started. You see what a mess it's making of by-name?

LOOK WHAT YOU MADE ME DO!

@ktonga
Copy link
Member

ktonga commented May 10, 2018

This looks as a good start although I recall seeing more differences when I applied the patch. I think we are missing the tests. Dunno if lines that were wrapped would rejoin or we need some sort of forcing flag for that to happen. Also we should turn off type tagging for vals in scalafix, that causes a lot of conflicts with code from sz8, mostly in the tests.

@fommil
Copy link
Contributor Author

fommil commented May 10, 2018

@ktonga maybe we should forward port the explicit types. We'd have to go through and remove them manually.

@ktonga
Copy link
Member

ktonga commented May 10, 2018

TBH annotating the tests with MatchResult seems useless and unfamiliar. I can do it as part of the next packport while i'm in merging party.

jmcardon
jmcardon approved these changes May 10, 2018
View reviewed changes
Copy link
Member

@jmcardon jmcardon left a comment

Choose a reason for hiding this comment

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

I for one welcome our new 120 col length overlords

@hrhino
Copy link
Member

hrhino commented May 10, 2018

@jmcardon confirmed for non-pureboi

@fommil fommil merged commit 6529156 into scalaz:master May 10, 2018
@fommil fommil deleted the formatting branch May 10, 2018 22:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@NeQuissimus NeQuissimus NeQuissimus approved these changes

@jmcardon jmcardon jmcardon approved these changes

@hrhino hrhino hrhino approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fommil @ktonga @hrhino @NeQuissimus @jmcardon