On Fri, Jul 17, 2020 at 8:06 AM Holden Karau <hol...@pigscanfly.ca> wrote:
> > > On Thu, Jul 16, 2020 at 3:34 PM Jungtaek Lim <kabhwan.opensou...@gmail.com> > wrote: > >> I agree with Wenchen that there are different topics. >> > I agree. I mentioned it in my postscript because I wanted to provide the > context around why I felt extra strongly about the importance of folks > following the explicit rules. It makes it feel like there is a double > standard, which is something I am not ok with. We've lost good community > participants over the appearance of a double standard, and I don’t think it > has to be intentional for us to loose people over this. > >> >> The policy of veto is obvious, as ASF doc describes it with explicitly >> saying non-overridable per project. In any way, the approach of resolving >> the situation should lead to voters withdrawing their vetoes. There's >> nothing to interpret differently, though I tend to agree with Sean's >> perspective, the one who vetoed the PR should indicate themselves as a >> blocker and follow up the PR actively on resolution of the veto (higher >> priority). Absence on a couple of days shouldn't matter, but what if it's >> going to be a couple of weeks, or even a couple of months, especially >> without any prior notices? >> >> "The unanimous opinion from the different folks I talked with is that >> any technical disagreement should be considered before moving on." >> >> This is not just for dealing with -1, but just for all of the reviews. +1 >> vote from any of committer without -1 vote from any qualified people (I >> guess it tends to be also interpreted as committer as well) is just a >> technical perspective. In practice, consensus should be made between >> reviewers to produce final approval on the PR. If there're non-trivial >> unresolved comments, merger has to stop and try to resolve these comments >> either letting the author to deal with them, or confirming from reviewers >> these comments are OK to defer/skip/take back. Obviously disagreements >> between reviewers must be sorted out. >> > So I believe, for example, if there is a new community member who places a > -1 they may not understand the significance of it. In those cases I always > seek to explain first, but I believe it's important to note that there is > some bar for what qualified is in terms of veto. > > It seems like we can agree that committers in our project are well above > the bar of a qualified voter for any code related discussion, and > certainly, there could be other community members who are qualified in > certain areas to veto as well. > >> >> PRs with someone asking to wait for certain days - how long the PR has >> been blocked by? That request may be the sign that they are voluntarily >> finding their time to review in detail, so it's not a bad thing. I think >> the real concern is that some days of silence in PR may lead to lose focus >> and let the PR be stale; but just a couple of days before certain days you >> can kindly remind, and after certain days you can go ahead, and they would >> go with post-hoc reviews if they have something to bring up. I don't see >> any issue here. >> > We're talking PRs open for > a month and PRs that have been open for > almost a quarter. It's fine to ask time to review, but I believe there is a > reasonable expectation that if someone cares about an area of review they > should be more active than taking a look once a month. > I believe I am one of the top strugglers of the Spark community for getting their PRs reviewed in recent 2 years. (Many of my PRs took more than 1 year to get reviewed and merged, and I still have a bunch of PRs posted over an year, with rebasing and triggering tests periodically.) I feel the frustration by heart. One thing I want to make sure of is that I don't mean "certain days" as months - I mean finding time in the next week or so. > This reminds me of the old way Spark said no to PRs by just ignoring them > which I think we agree is not the way we want to handle PRs anymore. > I agree showing disagreement might be better than ignoring, at least it leads to move forward, but you know, it's not that comfortable to show disagreement and open debate. It's not easy to force everyone to do so instead of simply ignoring. Not an easy problem. > >> On Thu, Jul 16, 2020 at 4:29 PM Wenchen Fan <cloud0...@gmail.com> wrote: >> >>> It looks like there are two topics: >>> 1. PRs with -1 >>> 2. PRs with someone asking to wait for certain days. >>> >>> Holden, it seems you are hitting 2? I think 2 can be problematic if >>> there are people who keep asking to wait, and block the PR indefinitely. >>> But if it's only asked once, this seems OK. BTW, since it's not a -1, so >>> policy-wise we can still merge the PR if we think the PR already gets >>> sufficient review. >>> >>> On Thu, Jul 16, 2020 at 4:33 AM Sean Owen <sro...@gmail.com> wrote: >>> >>>> I agree with all that, and would be surprised if anyone here objects >>>> to any of that in principle. In practice, I'm sure it doesn't end up >>>> that way sometimes, even in good faith. That is, I would not be >>>> surprised if the parties involved don't even see the disconnect. >>>> >>>> What are the specific examples? for private@ if necessary, but, I >>>> think dev@ could be fine because we're discussing general patterns >>>> from specific examples, for everyone to learn from. This isn't >>>> necessarily about individuals. (Heck, maybe I've gotten it wrong >>>> myself) >>>> >>>> One general principle I'd add: we are probably getting more >>>> conservative about big changes over time as Spark enters the long >>>> plateau of maturity. See the discussion about breaking changes in 3.0, >>>> or comments about waiting for review. That argues even more against >>>> proceeding against raised issues. >>>> >>>> On the flip side, we have to be constructive. I like the idea of >>>> proposing alternatives. Can you achieve this goal by doing Y instead >>>> of X? I also think there's a burden on the objector to provide a >>>> rationale, certainly, but also drive a resolution. That could also >>>> mean standing firm on the objection but calling in other reviewers and >>>> being willing to accede to a majority. Put another way: someone who >>>> objects and never really follows up with a path to consensus about >>>> compromise or rejection isn't really objecting correctly. We can VOTE >>>> if needed, but, if someone objected and didn't follow up and I >>>> couldn't find anyone else backing it up and thought I'd addressed the >>>> objection, I'd consider it resolved and proceed. >>>> >>>> >>>> >>>> On Wed, Jul 15, 2020 at 3:18 PM Holden Karau <hol...@pigscanfly.ca> >>>> wrote: >>>> > >>>> > Hi Spark Development Community, >>>> > >>>> > >>>> > Since Spark 3 has shipped I've been going through some of the PRs and >>>> I've noticed some PRs have been merged with pending -1s with technical >>>> reasons, including those from committers. I'm bringing this up because I >>>> believe we, as PMC, committers and contributors, do not currently have a >>>> consistent understanding, and I believe we should develop one. The >>>> foundation level guidance is at >>>> https://www.apache.org/foundation/voting.html#votes-on-code-modification >>>> . >>>> > >>>> > >>>> > >>>> > It is my belief that we should not be merging code with -1s from >>>> active committers, -1 is a very strong signal and generally a sign that we >>>> should step back and try and build consensus around the change. Looking at >>>> how the httpd project (e.g. the original Apache project) handles it >>>> https://httpd.apache.org/dev/guidelines.html#voting it seems to be >>>> that once a -1 from a committer is received we can no longer use our >>>> regular lazy consensus mechanism for the change, and we then need to have a >>>> vote or get the -1 resolved with the person who placed it. >>>> > >>>> > >>>> > >>>> > Now of course, if the -1s aren't following the guidelines that's a >>>> different story (e.g. no technical reason provided), but regardless a -1 >>>> from a committer to me would require a public vote on dev@ following >>>> the foundation's voting guidelines. >>>> > >>>> > >>>> > >>>> > I believe, especially in the Spark project, committers have >>>> demonstrated sufficient merit and they are qualified to vote on code >>>> changes so a -1 should block merging, however in talking with other ASF >>>> projects there are a variety of ways of handling this. The unanimous >>>> opinion from the different folks I talked with is that any technical >>>> disagreement should be considered before moving on. Just waiting a few days >>>> and merging the code is not a valid solution. In the context of how much >>>> work and history we've chosen to require Spark committers to demonstrate, >>>> most folks seem to believe that committers in the Spark project would be >>>> qualified voters for these purposes. >>>> > >>>> > >>>> > >>>> > In general I expect -1s to continue to be relatively rare in the >>>> Spark community, and if this is no longer the case I believe we should take >>>> the areas where we are seeing more -1s and have a broader discussion to >>>> give us the opportunity to build consensus prior to making any further >>>> non-bugfix changes in those components/areas. Most folks from other >>>> projects seemed to share this concern as well. >>>> > >>>> > >>>> > One of the things that was also brought up in this context is some >>>> projects require -1s to provide an alternative suggestion as well as the >>>> technical objection. That could be something we, as a project, could adopt >>>> if were concerned with over use of -1s. >>>> > >>>> > >>>> > >>>> > Sincerely, >>>> > >>>> > >>>> > >>>> > Holden >>>> > >>>> > >>>> > >>>> > P.S. >>>> > >>>> > >>>> > >>>> > I know this may be a sensitive topic, and we're all under more stress >>>> than usual right now. I appreciate everyone's patience while we discuss >>>> this. I know I find this a sensitive topic as well given the seemingly >>>> inconsistent standards. I'm tired of having people ask me to wait on PRs >>>> that have been open for more than a month and merging their own code with >>>> pending substantial issues raised by qualified members of the community. >>>> > >>>> > >>>> > >>>> > I'm not saying that the actions taken were necessarily wrong, just >>>> that I believe reaching a common understanding here is crucial to the >>>> healthy functioning of the project. >>>> > >>>> > >>>> > >>>> > I have not included any specific examples of this since this is the >>>> public list and I believe discussions involving individuals does not belong >>>> on dev@. If we want to discuss the specific situations that I noticed >>>> from the Spark 3 release, we can fork that conversation to private@. >>>> > >>>> > >>>> > >>>> > For any ASF members you can also find the discussion with folks from >>>> other projects and their views on that mailing list and get at the details. >>>> > >>>> > >>>> > >>>> > -- >>>> > Twitter: https://twitter.com/holdenkarau >>>> > Books (Learning Spark, High Performance Spark, etc.): >>>> https://amzn.to/2MaRAG9 >>>> > YouTube Live Streams: https://www.youtube.com/user/holdenkarau >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe e-mail: dev-unsubscr...@spark.apache.org >>>> >>>> > > -- > Twitter: https://twitter.com/holdenkarau > Books (Learning Spark, High Performance Spark, etc.): > https://amzn.to/2MaRAG9 <https://amzn.to/2MaRAG9> > YouTube Live Streams: https://www.youtube.com/user/holdenkarau > -- > Twitter: https://twitter.com/holdenkarau > Books (Learning Spark, High Performance Spark, etc.): > https://amzn.to/2MaRAG9 <https://amzn.to/2MaRAG9> > YouTube Live Streams: https://www.youtube.com/user/holdenkarau >
