spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Patrick Wendell <pwend...@gmail.com>
Subject Re: Scalastyle improvements / large code reformatting
Date Mon, 13 Oct 2014 05:16:52 GMT
Another big problem with these patches are that they make it almost
impossible to backport changes to older branches cleanly (there
becomes like 100% chance of a merge conflict).

One proposal is to do this:
1. We only consider new style rules at the end of a release cycle,
when there is the smallest chance of wanting to backport stuff.
2. We require that they are submitted in individual patches with a (a)
new style rule and (b) the associated changes. Then we can also
evaluate on a case-by-case basis how large the change is for each
rule. For rules that require sweeping changes across the codebase,
personally I'd vote against them. For rules like import ordering that
won't cause too much pain on the diff (it's pretty easy to deal with
those conflicts) I'd be okay with it.

If we went with this, we'd also have to warn people that we might not
accept new style rules if they are too costly to enforce. I'm guessing
people will still contribute even with those expectations.

- Patrick

On Sun, Oct 12, 2014 at 9:39 PM, Reynold Xin <rxin@databricks.com> wrote:
> I actually think we should just take the bite and follow through with the
> reformatting. Many rules are simply not possible to enforce only on deltas
> (e.g. import ordering).
>
> That said, maybe there are better windows to do this, e.g. during the QA
> period.
>
> On Sun, Oct 12, 2014 at 9:37 PM, Josh Rosen <rosenville@gmail.com> wrote:
>
>> There are a number of open pull requests that aim to extend Spark's
>> automated style checks (see
>> https://issues.apache.org/jira/browse/SPARK-3849 for an umbrella JIRA).
>> These fixes are mostly good, but I have some concerns about merging these
>> patches.  Several of these patches make large reformatting changes in
>> nearly every file of Spark, which makes it more difficult to use `git
>> blame` and has the potential to introduce merge conflicts with all open PRs
>> and all backport patches.
>>
>> I feel that most of the value of automated style-checks comes from
>> allowing reviewers/committers to focus on the technical content of pull
>> requests rather than their formatting.  My concern is that the convenience
>> added by these new style rules will not outweigh the other overheads that
>> these reformatting patches will create for the committers.
>>
>> If possible, it would be great if we could extend the style checker to
>> enforce the more stringent rules only for new code additions / deletions.
>> If not, I don't think that we should proceed with the reformatting.  Others
>> might disagree, though, so I welcome comments / discussion.
>>
>> - Josh

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org


Mime
View raw message