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

[SPARK-8660] [MLLib] removed > symbols from comments in LogisticRegressionSuite.scala for ease of copypaste #7167

Closed
wants to merge 9 commits into from

Conversation

Rosstin
Copy link
Contributor

@Rosstin Rosstin commented Jul 1, 2015

'>' symbols removed from comments in LogisticRegressionSuite.scala, for ease of copypaste

also single-lined the multiline commands (is this desirable, or does it violate style?)

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@Rosstin
Copy link
Contributor Author

Rosstin commented Jul 1, 2015

@mengxr Would it be desirable to un-multiline the LOC in the file's comments? Or should these remain multiline to follow style? (What I mean is, the lines are long enough that they were being broken into multiple lines, so copy-pasting them would be harder. I made them back into single-line.)

@holdenk
Copy link
Contributor

holdenk commented Jul 1, 2015

For copypasteing in Scala mode :paste mode makes the multi-line copy/past work well (although requires remembering that + ctrl-d)

@mengxr
Copy link
Contributor

mengxr commented Jul 1, 2015

Let's keep the line width within 100. As @holdenk mentioned, we can copy & paste a paragraph of code to Scala and ipython easily. I also tried RStudio, which takes multiline statement as well.

@Rosstin
Copy link
Contributor Author

Rosstin commented Jul 1, 2015

@mengxr @holdenk Alright, I restored the multiline comments conforming to the 100-character rule

@mengxr
Copy link
Contributor

mengxr commented Jul 1, 2015

ok to test

@mengxr
Copy link
Contributor

mengxr commented Jul 1, 2015

LGTM pending Jenkins.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jul 1, 2015

Test build #36316 has started for PR 7167 at commit f4b9bc8.

@SparkQA
Copy link

SparkQA commented Jul 2, 2015

Test build #36316 has finished for PR 7167 at commit f4b9bc8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@mengxr
Copy link
Contributor

mengxr commented Jul 2, 2015

Merged into master. Thanks!

@asfgit asfgit closed this in 4e4f74b Jul 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants