Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented Jan 12, 2016

Fix the style violation (space before , and :).
This PR is a followup for #10643

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, we also need to modify MimaExclude.scala.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we reset this change and just turn the rule off here-- otherwise the huge mima exclusion is pretty annoying.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's reasonable. I'll do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

So should we reset the change in flume/sink/Logging?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is less big of a deal there since it is already done and it is small. But feel free to do whatever you deem fit.

Copy link
Member Author

Choose a reason for hiding this comment

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

O.K, it's less big deal. I'll only reset for this change.

@SparkQA
Copy link

SparkQA commented Jan 12, 2016

Test build #49217 has finished for PR 10719 at commit e2202c8.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class ComplexFutureAction[T](run: JobSubmitter => Future[T])
    • class RangePartitioner[K: Ordering: ClassTag, V](
    • class SequenceFileRDDFunctions[K <% Writable: ClassTag, V <% Writable: ClassTag](
    • class OpenHashMap[K: ClassTag, @specialized(Long, Int, Double) V: ClassTag](
    • class KVArraySortDataFormat[K, T <: AnyRef: ClassTag] extends SortDataFormat[K, Array[T]]

@SparkQA
Copy link

SparkQA commented Jan 12, 2016

Test build #49221 has finished for PR 10719 at commit fc95915.

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

@sarutak sarutak force-pushed the SPARK-12692-followup-core branch 2 times, most recently from 5d4b1a9 to e953d13 Compare January 12, 2016 09:04
@SparkQA
Copy link

SparkQA commented Jan 12, 2016

Test build #49234 has finished for PR 10719 at commit e953d13.

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

@sarutak
Copy link
Member Author

sarutak commented Jan 12, 2016

The additional mima exclusions are few so I'll add them.

@SparkQA
Copy link

SparkQA commented Jan 12, 2016

Test build #49230 has finished for PR 10719 at commit 5d4b1a9.

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

@SparkQA
Copy link

SparkQA commented Jan 12, 2016

Test build #49236 has finished for PR 10719 at commit 9079c27.

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

@rxin
Copy link
Contributor

rxin commented Jan 12, 2016

I responded here: #10685 (comment)

Pasting the message here too:
@sarutak I actually reverted this change after talking more with Michael and looking at the diffs. The problem is that for colon, in some cases it is really useful to have a space before colon. In particular:

In many cases for type bounds, ti is good to have a space before the colon so it is more clear.
For type descriptions (e.g. variable : Int), it is also often useful to have a space.
I don't know whether it is feasible to actually implement the rules for the general colon, but exclude type description and type bounds. If yes, we should add this enforcement. Otherwise I'd prefer not to have it at all.

For comma, we should always have it (at least I haven't seen a case in which we don't want it yet).

@sarutak sarutak force-pushed the SPARK-12692-followup-core branch from 9079c27 to 284da4b Compare January 13, 2016 01:08
@sarutak sarutak changed the title [SPARK-12692][BUILD][CORE] Scala style: Fix the style violation (Space before "," or ":") [SPARK-12692][BUILD][CORE] Scala style: Fix the style violation (Space before ",") Jan 13, 2016
@SparkQA
Copy link

SparkQA commented Jan 13, 2016

Test build #49280 has finished for PR 10719 at commit 284da4b.

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

@rxin
Copy link
Contributor

rxin commented Jan 13, 2016

I've merged this.

@asfgit asfgit closed this in f14922c Jan 13, 2016
@sarutak sarutak deleted the SPARK-12692-followup-core branch April 12, 2016 13:58
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