spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Erik Erlandson <...@redhat.com>
Subject Re: Scalastyle improvements / large code reformatting
Date Mon, 13 Oct 2014 17:27:47 GMT


----- Original Message -----
> I'm also against these huge reformattings. They slow down development and
> backporting for trivial reasons. Let's not do that at this point, the style
> of the current code is quite consistent and we have plenty of other things
> to worry about. Instead, what you can do is as you edit a file when you're
> working on a feature, fix up style issues you see. Or, as Josh suggested,
> some way to make this apply only to new files would help.


+1, the benefit/cost ratio of wide-spread code churn just to alter formatting is close to
zero.



> 
> Matei
> 
> On Oct 12, 2014, at 10:16 PM, Patrick Wendell <pwendell@gmail.com> wrote:
> 
> > 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
> > 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
> For additional commands, e-mail: dev-help@spark.apache.org
> 
> 

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


Mime
View raw message