On May 24, 2016, at 6:49 PM, Glyph wrote:

> I KNOW, RIGHT!!!  However, protected statuses somewhat reduce the potential 
> race-condition here.  And contributing to a couple dozen Github projects I 
> have to say that practically this has never been an issue, even though I find 
> it aesthetically appalling, design-wise.

> But sometimes people make a fork and their fork branch just happens to be 
> called 'patch-1', or 'master',  this doesn't really affect our workflow as 
> long as they observe proper PR etiquette and don't push any unrelated 
> revisions to that branch in the meanwhile.

In terms of "git flow" i didn't mean the exact names, just the concept that 
every fix has it's own dedicated branch. This is something that almost every 
github-experienced person does automatically (and all current twisted PRs do).  
People with less experience on github itself (not a given package) will often 
just edit the "master" branch and submit.  then they'll forget they did that 
and edit something else... because of that github peculiarity, and that 
everything is acting "centralized" on github and not under a "decentralized" 
model where a particular commit was queued that ends up in the review.  those 
types of changes can also end up triggering CI tests, which complicates things 
further.

twisted contributors are likely a bit more experienced and it's a 
self-selecting pool... i've just seen an increasing number of projects require 
a dedicated branch and politely reject + ask for anything against 'master' to 
be resubmitted via a dedicated branch.

> I don't see how it's incompatible.  It seems perfectly fine, as long as we 
> shift from 'review queue == report 25' to 'review queue == open PRs with 
> non-failing status'.

ah, I did not know that was possible.  this ties into me not understanding the 
content of the queue being PR only (not "all tickets").

>> At least on the github side, using some sort of label to state where in the 
>> process (ie, what type of review) something is under would help.
> 
> What do you mean it "would help"?  What problem would it solve?

This would help non-maintainers understand what that actual progress was in 
that 5 stage review.

> 
>> The caveat is that this breaks the current use of Issue as a "queue" item.  
>> But Twisted is using "issues" less as an Issue and more as a queue item, 
>> while github "Issues" are less of a queue item and amore of an actual issue. 
>>  This is a bit confusing and different to how many people will use github.
> 
> I don't know where you're getting this from.  Only tickets in review are 
> queue items.  I have no idea what "actual issue" means here.

Ok. From here to the end I believe I understand my confusion.  The "queue" 
looked to me like it was "All Trac Tickets" not just "Certain types of Trac 
Tickets".   I was looking at a few reports and some raw views that showed track 
tickets that were both bug reports and attempts to fix a specific problem.  It 
looked like the normal use of github would have run antagonistic to your 
workflow , but his all seems fine now.

> The practical issue I'm trying to address by switching to github PRs as 
> opposed to trac tickets with the 'review' keyword is the speed and ease of 
> submitting a change and getting CI feedback on that change, in addition to 
> lowering training overhead since many people are familiar with github.  
> Faster feedback = happier contributors = more maintenance effort productively 
> expended.  The issue I'm trying to address with the slightly odd 
> close-a-PR-to-signify-finished-review workflow is to avoid the common problem 
> of reviewers not knowing which things to review and therefore leaving certain 
> contributions mouldering until their submitters lose interest.  Beyond that, 
> I don't see that we have substantial workflow issues that need solving.

I was mostly concerned with the proposed implementation creating workflow 
issues.




_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Reply via email to