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
