Skip to content

Conversation

@kiszk
Copy link
Member

@kiszk kiszk commented Dec 30, 2015

Use multi-line string literals for @ExpressionDescription with // scalastyle:off line.size.limit and // scalastyle:on line.size.limit

The policy is here, as describe at #10488

Let's use multi-line string literals. If we have to have a line with more than 100 characters, let's use // scalastyle:off line.size.limit and // scalastyle:on line.size.limit to just bypass the line number requirement.

@kiszk
Copy link
Member Author

kiszk commented Dec 30, 2015

@yhuai, are these changes fine with the policy that you proposed? I would appreciate it if you would check them.

@rxin
Copy link
Contributor

rxin commented Dec 30, 2015

Actually this line length makes it pretty annoying to view. Can't we wrap them, and just require two newlines for a real newline?

@SparkQA
Copy link

SparkQA commented Dec 30, 2015

Test build #48462 has finished for PR 10524 at commit acb422c.

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

@SparkQA
Copy link

SparkQA commented Dec 30, 2015

Test build #48466 has finished for PR 10524 at commit 193568c.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member Author

kiszk commented Dec 30, 2015

On jenkins with scala-2.11, we can wrap once in a source file (i.e. only one string concatenation). To use more than one concatenation causes compilation error.
Is it OK to use only one string concatenation?

@kiszk
Copy link
Member Author

kiszk commented Dec 30, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Dec 30, 2015

Test build #48490 has finished for PR 10524 at commit 193568c.

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

@yhuai
Copy link
Contributor

yhuai commented Dec 30, 2015

@kiszk Can you try

"""
1st line

2nd line

3rd line
"""

and see if it works?

@kiszk
Copy link
Member Author

kiszk commented Dec 31, 2015

@yhuai, I think that I will work. Do you think how many characters are preferable to all at a line?

@yhuai
Copy link
Contributor

yhuai commented Jan 4, 2016

How about 80 characters? Let's make sure it is easy to read by users (who use describe function command) and developers (who read code).

@kiszk
Copy link
Member Author

kiszk commented Jan 4, 2016

I see. Sounds good. I will reformat them.

@SparkQA
Copy link

SparkQA commented Jan 4, 2016

Test build #48670 has finished for PR 10524 at commit 19bb151.

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

@SparkQA
Copy link

SparkQA commented Jan 4, 2016

Test build #48671 has finished for PR 10524 at commit f2c1094.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member Author

kiszk commented Jan 5, 2016

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Jan 5, 2016

Test build #48717 has finished for PR 10524 at commit f2c1094.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this unchanged? The indentation looks weird.

@SparkQA
Copy link

SparkQA commented Jan 7, 2016

Test build #48941 has finished for PR 10524 at commit 4649202.

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

@yhuai
Copy link
Contributor

yhuai commented Jan 7, 2016

thanks! Merging to master.

@asfgit asfgit closed this in 34dbc8a Jan 7, 2016
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