----- 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 <pwend...@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 <r...@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 <rosenvi...@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-unsubscr...@spark.apache.org > > For additional commands, e-mail: dev-h...@spark.apache.org > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@spark.apache.org > For additional commands, e-mail: dev-h...@spark.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@spark.apache.org For additional commands, e-mail: dev-h...@spark.apache.org
