Re: [Twisted-Python] overview: new review queue venue

[Glyph]

> On May 21, 2016, at 11:36 PM, Amber Hawkie Brown  
> wrote:
> 
> 
>> On 22 May 2016, at 14:32, Glyph  wrote:
>> 
>> 
>>> On May 21, 2016, at 11:21 PM, Amber Hawkie Brown 
>>>  wrote:
>>> 
>>> 
 On 22 May 2016, at 14:15, Glyph  wrote:
 
 Sorry, I guess I wasn't clear.  I know that PRs are presently a potential 
 alternative to a diff, and that we are still using Trac for ticketing.  I 
 want to make it possible to avoid using Trac for ticketing; perhaps 
 switching to github issues entirely.
>>> 
>>> This is an optimistic idea but one that, unfortunately, won't happen yet ;)
>>> 
>>> The things GitHub Issues need to be competitive with Trac as it stands:
>>> 
>>> - Allowing triage by people without write.
>> 
>> How are things currently "triaged"?  Do you mean "review"?  If so, I think 
>> it would be acceptable to come up with a magic comment for a non-commiter to 
>> use to signify that they've fully reviewed a PR.  As it stands, we need 
>> committers to "accept" a review by deciding to merge, the only difference 
>> here is that it would remain in the review queue until they did so, which I 
>> think is acceptable (since if the review isn't accepted, it should have 
>> remained in the queue anyway).  We could also have a bot address this 
>> edge-case somehow.
> 
> Creating a ticket, adding it to the relevant milestone (commit required on 
> GitHub), setting the component (which would be a tag on github, requires 
> commit)...

I don't see us getting a lot of non-committer triage of this kind.  And I'm not 
sure that's really an important part of our workflow - do you really feel that 
it is?  Frankly when non-committers try to put their changes into a milestone 
or start adding custom keywords, it's almost always wrong.

We also don't use "component" for much.  If we just got rid of it, would any 
part of our process change?

>>> - Useful search (GitHub search is kind of abysmal)
>> 
>> I don't see how Trac's is better.
> 
> It has a GUI rather than being stringly typed ;)

Oh, you're talking about "query", i.e. https://twistedmatrix.com/trac/query, 
not "search", i.e. https://twistedmatrix.com/trac/search ?

GitHub's search is actually structured, not stringly typed; there's a bit of 
GUI here: https://github.com/search/advanced.  It does resolve down to a string 
query, but so does the trac "custom query" eventually (in that it goes into a 
URL you can copy and paste).

But, this is all sort of abstract: what kinds of queries would you do against 
our ticket database in Trac that are not possible in GitHub's query language?  
Personally after some exploration of GitHub's query language I have found it's 
actually more expressive for what I want to do most of the time; particularly 
querying across projects which is obviously not possible with trac...

>>> - Assigning to non-committers.
>> 
>> Honestly I'm not sure that the non-committer assignment part of the workflow 
>> is all that useful.  I know I hardly ever look at report 7, and I very much 
>> doubt any non-committer does :).  It's not like we're losing information, 
>> either; we still have a record of whose fork the PR points to.
> 
> I look at report 7 :(

That's not a counterexample: you are a committer :-).

I do actually have a recurring personal to-do item to check report 7 nowadays, 
but almost all of my doing-stuff-on-the-tracker has to do with me getting 
emails about actionable state changes (somebody reviewed your change, you 
should merge it), rather than scanning that list.

>>> Without these things (and quite a few more), it's unlikely that GitHub 
>>> Issues will be as useful to us.
>> 
>> I am curious about the "quite a few more".  There are things which we really 
>> need as a critical part of our workflow (primarily: the review queue) and 
>> then there are accidents of the way trac works.  Nothing is graven in stone 
>> here :).
> 
> I guess there's a lot of things that are an accident of trac, but the things 
> above are useful.

I'm not disputing that, but I'm still a little confused about how exactly 
they're useful, rather than just different.  Can you give some examples of 
things you do regularly?

-glyph

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

Re: [Twisted-Python] overview: new review queue venue

[Hynek Schlawack]

>>> A lot of projects do follow this workflow, and maybe it will be fine for 
>>> us.  The real question is; is FreeBSD support really worth it for the cost 
>>> to contributors, since that's the only platform we currently support but 
>>> can't test?
>> 
>> I'm guessing that we have more FreeBSD users than Windows users ;)
> 
> 
> I realize it can feel like that sometimes, but Google Analytics suggests the 
> large majority of our visitors (45%) are on Windows.  By contrast, 0.05% are 
> on FreeBSD.  Granted, that's a very high percentage of FreeBSD clients for 
> the Internet at large, but nevertheless, I think your perspective may be 
> slightly statistically skewed.

I don’t think there’s a meaningful correlation between what people use to 
browse docs vs. what people run Twisted on.

E.g. Netflix & WhatsApp run on FreeBSD; do you think their tech staff has 
FreeBSD on their desktops? :)

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

Re: [Twisted-Python] overview: new review queue venue

[Hynek Schlawack]

Ah finally a fine bike shedding thread that gets everyone involved. ;)

> Right now, we need to manually vet each change before sending it to 
> buildbots, because they are shared mutable environments that we can't afford 
> to have running untrusted code automatically.  If we could switch to Travis 
> and Appveyor, then we could let them worry about malicious code, which would 
> allow contributors to get instant feedback, rather than waiting for reviewers 
> to manually run the builders.

I have two points to add:

1. Appveyor is terribly slow and sometimes a bit flaky.  I use it for 
argon2_cffi’s wheels and it drives me bonkers.  It should never become an 
essential part of anything.  As a first line of defense it’s fine of course.
2. PyCA has a workflow for Jenkins & GitHub by telling a bot to vet changes.  
You can see it here in action: 
https://github.com/pyca/cryptography/pull/2914#issuecomment-220592167 AFAIK 
that’s been mostly Paul’s work.  Aren’t you kind of his boss or something *hint 
hint*? ;)
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] overview: new review queue venue

[Glyph]

> On May 22, 2016, at 12:09 AM, Hynek Schlawack  wrote:
> 
> 
 A lot of projects do follow this workflow, and maybe it will be fine for 
 us.  The real question is; is FreeBSD support really worth it for the cost 
 to contributors, since that's the only platform we currently support but 
 can't test?
>>> 
>>> I'm guessing that we have more FreeBSD users than Windows users ;)
>> 
>> 
>> I realize it can feel like that sometimes, but Google Analytics suggests the 
>> large majority of our visitors (45%) are on Windows.  By contrast, 0.05% are 
>> on FreeBSD.  Granted, that's a very high percentage of FreeBSD clients for 
>> the Internet at large, but nevertheless, I think your perspective may be 
>> slightly statistically skewed.
> 
> I don’t think there’s a meaningful correlation between what people use to 
> browse docs vs. what people run Twisted on.
> 
> E.g. Netflix & WhatsApp run on FreeBSD; do you think their tech staff has 
> FreeBSD on their desktops? :)

The existence of Netflix & WhatsApp doesn't indicate no correlation, just not a 
perfect correlation :).  Although I suppose I'm being unfair there in that some 
of the Windows users represent actual FreeBSD users as well.

Nevertheless, I think Windows consumers are less frequently acculturated to 
speaking up in open-source land, so if anything we are underestimating their 
prevalence.

Unrelatedly, the existence of such high-profile FreeBSD users is encouraging 
:).  Perhaps this is more important to keep in the critical path than I was 
assuming.

-glyph

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

Re: [Twisted-Python] overview: new review queue venue

[Glyph]

> On May 22, 2016, at 12:15 AM, Hynek Schlawack  wrote:
> 
> Ah finally a fine bike shedding thread that gets everyone involved. ;)
> 
>> Right now, we need to manually vet each change before sending it to 
>> buildbots, because they are shared mutable environments that we can't afford 
>> to have running untrusted code automatically.  If we could switch to Travis 
>> and Appveyor, then we could let them worry about malicious code, which would 
>> allow contributors to get instant feedback, rather than waiting for 
>> reviewers to manually run the builders.
> 
> I have two points to add:
> 
> 1. Appveyor is terribly slow and sometimes a bit flaky.  I use it for 
> argon2_cffi’s wheels and it drives me bonkers.  It should never become an 
> essential part of anything.  As a first line of defense it’s fine of course.

This is a very useful data point.  I do not have any concrete experience with 
it and I was kind of wondering about this.

> 2. PyCA has a workflow for Jenkins & GitHub by telling a bot to vet changes.  
> You can see it here in action: 
> https://github.com/pyca/cryptography/pull/2914#issuecomment-220592167 AFAIK 
> that’s been mostly Paul’s work.  Aren’t you kind of his boss or something 
> *hint hint*? ;)

Thanks for the promotion; I'll be sure to let him know on Monday.

However, it's because I know Paul and I know what a complete nightmare it is to 
set up and maintain infrastructure like that that I was hoping to cheat and get 
away with it.  But what I'm hearing from you in this thread is pretty 
compelling to me that we are going to need to follow a mostly Cryptography-like 
workflow after all, asking a bot to run some buildbots.

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

Re: [Twisted-Python] overview: new review queue venue

[Craig Rodrigues]

On Sat, May 21, 2016 at 6:04 PM, Glyph  wrote:

>
> Right now, we need to manually vet each change before sending it to
> buildbots, because they are shared mutable environments that we can't
> afford to have running untrusted code automatically.
>

This is quite useful actually.  We would need a tool to do this.

For example, if I want to build this pr:
https://github.com/twisted/twisted/pull/63

Then the tool could poke the buildbots to do something like:

git clone https://github.com/twisted/twisted testspace
cd testspace
git fetch origin pull/62/head:pr/62
git checkout pr/62
[run the tests]

Are there enough scripts in the buildbot infrastructure which could be
extended to do this?

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

Re: [Twisted-Python] overview: new review queue venue

[Glyph]

> On May 22, 2016, at 12:15 AM, Hynek Schlawack  wrote:
> 
> Ah finally a fine bike shedding thread that gets everyone involved. ;)

OKAY NOW THAT I'VE GOT YOU ALL HERE LET'S TALK ABOUT 
https://twistedmatrix.com/trac/ticket/288

*slams a metal grating shut over the only exit from the mailing list*

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

Re: [Twisted-Python] overview: new review queue venue

[Amber "Hawkie" Brown]

> On 22 May 2016, at 15:23, Glyph  wrote:
> 
> 
>> On May 22, 2016, at 12:15 AM, Hynek Schlawack  wrote:
>> 
>> Ah finally a fine bike shedding thread that gets everyone involved. ;)
> 
> OKAY NOW THAT I'VE GOT YOU ALL HERE LET'S TALK ABOUT 
> https://twistedmatrix.com/trac/ticket/288
> 
> *slams a metal grating shut over the only exit from the mailing list*
> 
JEAN-CHA^W^W GLYPH YOU FOOL, YOU'VE LOCKED US ALL IN!

> -glyph


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] overview: new review queue venue

[Glyph]

> On May 22, 2016, at 12:24 AM, Craig Rodrigues  wrote:
> 
> On Sat, May 21, 2016 at 6:04 PM, Glyph  > wrote:
> 
> Right now, we need to manually vet each change before sending it to 
> buildbots, because they are shared mutable environments that we can't afford 
> to have running untrusted code automatically.
> 
> This is quite useful actually.  We would need a tool to do this.
> 
> For example, if I want to build this pr:   
> https://github.com/twisted/twisted/pull/63 
> 
> 
> Then the tool could poke the buildbots to do something like:
> 
> git clone https://github.com/twisted/twisted 
>  testspace
> cd testspace
> git fetch origin pull/62/head:pr/62
> git checkout pr/62
> [run the tests]
> 
> Are there enough scripts in the buildbot infrastructure which could be 
> extended to do this?

The only new line would be fetching the test ref.  Everything else on the 
buildbots basically works that way already, just checking out branches.

(Please nobody try to do the clever thing where you configure buildbot to 
automatically pull all PRs, that would effectively negate any security 
protections...)

I've been assuming that in the worst-case scenario, we'd do what Cryptography 
does and have a bot that polls for special comments and then triggers buildbot 
in exactly this way.  Perhaps I should have made that assumption explicit, I 
thought it was ticketed somewhere in Braid already.

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

Re: [Twisted-Python] overview: new review queue venue

[Ralph Meijer]

On May 22, 2016 9:36:28 AM GMT+02:00, Glyph  wrote:
>[..]
>(Please nobody try to do the clever thing where you configure buildbot
>to automatically pull all PRs, that would effectively negate any
>security protections...)
>
>I've been assuming that in the worst-case scenario, we'd do what
>Cryptography does and have a bot that polls for special comments and
>then triggers buildbot in exactly this way.  Perhaps I should have made
>that assumption explicit, I thought it was ticketed somewhere in Braid
>already.

The Jenkins plugin for GitHub PR triggers has this feature, too. However, it 
also has a feature to whitelist users and GitHub teams so that PRs/commits can 
trigger automatically for them. Maybe that's a thing for us, too?


-- 
ralphm

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

Re: [Twisted-Python] overview: new review queue venue

[Tristan Seligmann]

On Sun, 22 May 2016 at 10:12 Ralph Meijer  wrote:

> On May 22, 2016 9:36:28 AM GMT+02:00, Glyph 
> wrote:
> >[..]
> >(Please nobody try to do the clever thing where you configure buildbot
> >to automatically pull all PRs, that would effectively negate any
> >security protections...)
> >
> >I've been assuming that in the worst-case scenario, we'd do what
> >Cryptography does and have a bot that polls for special comments and
> >then triggers buildbot in exactly this way.  Perhaps I should have made
> >that assumption explicit, I thought it was ticketed somewhere in Braid
> >already.
>
> The Jenkins plugin for GitHub PR triggers has this feature, too. However,
> it also has a feature to whitelist users and GitHub teams so that
> PRs/commits can trigger automatically for them. Maybe that's a thing for
> us, too?
>

I don't think we need a whitelist, we can just automatically build branches
that are pushed to the twisted/twisted repository. If you can push a branch
there, you can also push a change directly to trunk, so you can already
execute arbitrary code on the buildbots.
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] overview: new review queue venue

[Clayton Daley]

>
> The thing is, if you perceive it as "hostile" that a project closes a PR -
> i.e. "says that they're not going to do more work on it" - that is a
> cultural problem; it suggests a certain implicit level of passive
> aggression in opening a PR which I don't want to assume.  It's sort of like
> having a culture where you can just send anybody an email asking them to do
> whatever and it would be "hostile" for them to refuse.  In such a culture
> people don't say "yes", but they do start to ignore messages  Closing the
> PR is a more accurate reflection of *reality* - the project (twisted) is
> not going to do anything about the PR in its current state, so it shouldn't
> be left open.  It also clearly demarcates the completion of a review.
>
...

I'd much rather our new contributors get a little confused about the
> culturally unusual step of closing a PR than to have their work be
> accidentally but systematically discriminated against in favor of people
> who know how to bug the right people in IRC or email.
>

It's your party, but I think this vastly undervalues first impressions in
OSS engagement.  From all the projects I've contributed to on Github -- yes
technically Github not Git though Bitbucket has equivalent features (and
unlimited private repos for free!) -- closing means "this is off the
table".  It's a nice clear signal that there's nothing the contributor can
do to fix it -- e.g. I had an LSP-valid change that passed all tests run
afoul of some obscure PHP method signature limitation when mixed with other
packages.

I'll wager only the active contributors will remember the proposed subtlety
a month from now.  As a big break from Github norms, It's going to hit
everyone else at the worst time... a first PR submission.  That's the
moment when a contributor is trying to get a sense of the culture of the
team that manages the project.  They're at their most vulnerable and
violating *their* norms significantly increases the odds that they'll leave
the fix in a private fork and disengage.

Twisted operates at a different level so this may not be a bad thing.  You
may benefit from actively discouraging dabblers -- especially given
resource constraints.  But there aren't going to be a lot of "first PR"
folks on this list to point out the effect of this break from norms.

Clayton Daley

On Sun, May 22, 2016 at 12:12 AM, Glyph  wrote:

>
> On May 21, 2016, at 7:25 PM, Clayton Daley 
> wrote:
>
> To qualify my comments, I've yet to contribute to Twisted because I don't
> have a good enough grasp of its internals, but I have contributed to a
> variety of Git-based OSS projects.  I definitely get uneasy with the
> general idea that we're trying to "replicate workflow A from Trac in
> tangentially related Git PR feature".
>
>
> The workflow is not "from Trac".  The instantiation in Trac is not optimal
> either, which is why I described the abstract desired workflow separately
> from our existing instantiation.
>
> We're in Git. We're hoping to solicit PRs from Git users. Git users will
> be used to the way PRs are used in other OSS Git projects.
>
>
> I think you mean "GitHub".  Git PRs don't work at *all* like GitHub PRs.
> :).
>
> Glyph has some valid criticisms of the situation in some projects, but it
> should still be the starting point. For example, closing a PR strikes me as
> a bad idea -- for lack of a better word, it feels "hostile" to me and
> certainly unwelcoming.
>
>
> The thing is, if you perceive it as "hostile" that a project closes a PR -
> i.e. "says that they're not going to do more work on it" - that is a
> cultural problem; it suggests a certain implicit level of passive
> aggression in opening a PR which I don't want to assume.  It's sort of like
> having a culture where you can just send anybody an email asking them to do
> whatever and it would be "hostile" for them to refuse.  In such a culture
> people don't say "yes", but they do start to ignore messages  Closing the
> PR is a more accurate reflection of *reality* - the project (twisted) is
> not going to do anything about the PR in its current state, so it shouldn't
> be left open.  It also clearly demarcates the completion of a review.
>
> People feel very differently about workflow, of course, but I've
> definitely heard from other OSS maintainers that the average workflow of
> volunteer projects often devolves into a huge backlog of un-reviewed stuff,
> which obscures the new stuff, and if you want something to actually get
> reviewed and move along you need to know the maintainers of the project and
> ask them personally.
>
> I'd much rather our new contributors get a little confused about the
> culturally unusual step of closing a PR than to have their work be
> accidentally but systematically discriminated against in favor of people
> who know how to bug the right people in IRC or email.
>
> In several of the projects I've seen, Git tags fill these roles. Piwik has
> a "needs review" tag -- the short list for reviewers. 

Re: [Twisted-Python] overview: new review queue venue

[Glyph]

> On May 22, 2016, at 6:18 AM, Clayton Daley  wrote:
> 
> The thing is, if you perceive it as "hostile" that a project closes a PR - 
> i.e. "says that they're not going to do more work on it" - that is a cultural 
> problem; it suggests a certain implicit level of passive aggression in 
> opening a PR which I don't want to assume.  It's sort of like having a 
> culture where you can just send anybody an email asking them to do whatever 
> and it would be "hostile" for them to refuse.  In such a culture people don't 
> say "yes", but they do start to ignore messages  Closing the PR is a more 
> accurate reflection of reality - the project (twisted) is not going to do 
> anything about the PR in its current state, so it shouldn't be left open.  It 
> also clearly demarcates the completion of a review.
> ... 
> I'd much rather our new contributors get a little confused about the 
> culturally unusual step of closing a PR than to have their work be 
> accidentally but systematically discriminated against in favor of people who 
> know how to bug the right people in IRC or email.
> 
> It's your party, but I think this vastly undervalues first impressions in OSS 
> engagement.  From all the projects I've contributed to on Github -- yes 
> technically Github not Git though Bitbucket has equivalent features (and 
> unlimited private repos for free!) -- closing means "this is off the table".  
> It's a nice clear signal that there's nothing the contributor can do to fix 
> it -- e.g. I had an LSP-valid change that passed all tests run afoul of some 
> obscure PHP method signature limitation when mixed with other packages.
>  
> I'll wager only the active contributors will remember the proposed subtlety a 
> month from now.  As a big break from Github norms, It's going to hit everyone 
> else at the worst time... a first PR submission.  That's the moment when a 
> contributor is trying to get a sense of the culture of the team that manages 
> the project.  They're at their most vulnerable and violating *their* norms 
> significantly increases the odds that they'll leave the fix in a private fork 
> and disengage.
> 
> Twisted operates at a different level so this may not be a bad thing.  You 
> may benefit from actively discouraging dabblers -- especially given resource 
> constraints.  But there aren't going to be a lot of "first PR" folks on this 
> list to point out the effect of this break from norms.

This is roughly the same story I've been getting for the last decade and a half:

"You can't:

require test coverage,
require documentation,
require coding standard compliance,
require people to file a ticket before sending a patch to the mailing list,

that's a terrible burden to put on new contributors!"

Somehow we've survived much longer than most projects, and while some would say 
"in spite of" these restrictions, I think it's "because of".  So, we are not 
trying to "discourage dabblers"; we would like new contributors who want to 
contribute only a little bit.  So while I don't want to throw up arbitrary 
barriers, if you aren't willing to invest the effort to even read a single 
comment on your PR explaining why it was closed and how to reopen it, I cannot 
imagine that chances are good that you'll read the rest of the comments 
explaining what changes need to be made and make them effectively.

Further, people who contribute trivial changes that can be immediately merged, 
like documentation typos, won't need to deal with this, because they won't see 
the intermediary "needs feedback" closed state; they'll just get their PRs 
accepted immediately.

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

Re: [Twisted-Python] overview: new review queue venue

[Clayton Daley]

>
> "You can't:
>
>- require test coverage,
>- require documentation,
>- require coding standard compliance,
>- require people to file a ticket before sending a patch to the
>mailing list,
>
> The first three of these are *already* norms in all of the OSS projects of
this caliber and the 4th doesn't make sense in a Github/Bitbucket/Gitlab
world (and I fortunately avoided its predecessors).  In fact, I'd be a
little worried if the first there weren't required in a project.
ResourceSpace is the only one I can think of that doesn't require them and
I wouldn't touch it with a 10 foot pole if there were acceptable (free)
alternatives.  So I don't see the link between your success enforcing three
norms of good OSS projects and your desire to break a 4th.

===

The idea that PRs are a substantial burden has caused me pause.  Normally,
a PR is a chance to give back to a project rather than freeload.  In most
projects, new features are part of a virtuous cycle:  a new feature tends
to benefit a large fraction of the user base and expand the user base...
which draws more contributors.

Especially given the recent deprecations (old, unmaintained protocols), it
seems that Twisted doesn't work like this.  If another user adds another
protocol, it doesn't make the system better for most users.  In fact, it
makes sense that it actually increases the maintenance burden.

I tried to look for myself, but was reminded that I don't know the
internals well enough... are patches on non-central protocols a big part of
the backlog?  Or is the backlog mostly core features (like reactors or IO
infrastructure) that most projects depend upon?

Clayton Daley

On Sun, May 22, 2016 at 4:22 PM, Glyph  wrote:

>
> On May 22, 2016, at 6:18 AM, Clayton Daley 
> wrote:
>
> The thing is, if you perceive it as "hostile" that a project closes a PR -
>> i.e. "says that they're not going to do more work on it" - that is a
>> cultural problem; it suggests a certain implicit level of passive
>> aggression in opening a PR which I don't want to assume.  It's sort of like
>> having a culture where you can just send anybody an email asking them to do
>> whatever and it would be "hostile" for them to refuse.  In such a culture
>> people don't say "yes", but they do start to ignore messages  Closing the
>> PR is a more accurate reflection of *reality* - the project (twisted) is
>> not going to do anything about the PR in its current state, so it shouldn't
>> be left open.  It also clearly demarcates the completion of a review.
>>
> ...
>
> I'd much rather our new contributors get a little confused about the
>> culturally unusual step of closing a PR than to have their work be
>> accidentally but systematically discriminated against in favor of people
>> who know how to bug the right people in IRC or email.
>>
>
> It's your party, but I think this vastly undervalues first impressions in
> OSS engagement.  From all the projects I've contributed to on Github -- yes
> technically Github not Git though Bitbucket has equivalent features (and
> unlimited private repos for free!) -- closing means "this is off the
> table".  It's a nice clear signal that there's nothing the contributor can
> do to fix it -- e.g. I had an LSP-valid change that passed all tests run
> afoul of some obscure PHP method signature limitation when mixed with other
> packages.
>
> I'll wager only the active contributors will remember the proposed
> subtlety a month from now.  As a big break from Github norms, It's going to
> hit everyone else at the worst time... a first PR submission.  That's the
> moment when a contributor is trying to get a sense of the culture of the
> team that manages the project.  They're at their most vulnerable and
> violating *their* norms significantly increases the odds that they'll leave
> the fix in a private fork and disengage.
>
> Twisted operates at a different level so this may not be a bad thing.  You
> may benefit from actively discouraging dabblers -- especially given
> resource constraints.  But there aren't going to be a lot of "first PR"
> folks on this list to point out the effect of this break from norms.
>
>
> This is roughly the same story I've been getting for the last decade and a
> half:
>
> "You can't:
>
>
>- require test coverage,
>- require documentation,
>- require coding standard compliance,
>- require people to file a ticket before sending a patch to the
>mailing list,
>
>
> that's a terrible burden to put on new contributors!"
>
> Somehow we've survived much longer than most projects, and while some
> would say "in spite of" these restrictions, I think it's "because of".  So,
> we are not trying to "discourage dabblers"; we would like new contributors
> who want to contribute only a little bit.  So while I don't want to throw
> up arbitrary barriers, if you aren't willing to invest the effort to even
> read a single comment on your PR explaining why it was closed and how to
> reopen it, I cannot

Re: [Twisted-Python] overview: new review queue venue

[Donald Stufft]

Twisted has been enforcing these rules since before they were considered part 
of the norm and I believe that Glyph was referencing is that back then people 
said that Twisted was going to fail or w/e because of requiring those things. 

Sent from my iPhone

> On May 22, 2016, at 9:12 PM, Clayton Daley  wrote:
> 
> The first three of these are *already* norms in all of the OSS projects of 
> this caliber


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

Re: [Twisted-Python] overview: new review queue venue

[meejah]

Glyph  writes:

> This is a very useful data point.  I do not have any concrete
> experience with it and I was kind of wondering about this.

FWIW, Tahoe-LAFS *just* started using AppVeyor too, and I also find it
horrifically slow. That said, the Tahoe tests run pretty slowly on a
VirtualBox windows VM as well, but not nearly as slowly as AppVeyor. Or,
at least that's my impression so far.

p.s. If anyone is interested in running a Windows buildbot slave for
Tahoe I'm very sure it would be appreciated -- please speak up on the
tahoe mailing list :) (my understanding of the setup is that the
buildbot slaves don't run random PR code, only post-merged-to-master
code).

-- 
meejah

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

Re: [Twisted-Python] overview: new review queue venue

[meejah]

Personally, I find closing PRs that aren't going to be merged "soon"
mostly-beneficial. Even if it *might* be perceived as "hostile" by some
contributers, a simple explanation should suffice. (And, if simply
closing a PR with a nice note explaining, "please re-open when X is
fixed/changed" scares away a potential contributer I have my doubts as
to whether they would fix X if you *didn't* close it...)

There's nothing worse than trolling through open PRs only to find that
the last comment is "fix up X, Y, and Z and we'll merge" because then
you have to (re-)figure out if X, Y and Z have been done etc. On the
flip side, it's nice to know if your PR has problems or not.

The other plus of closing is that it's way more obvious when the PR is
once again considered ready for merging (even without 'completely baked'
workflows like Twisted's) and keeps the "open PRs" list (hopefully!)
shorter.

-- 
meejah

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

Re: [Twisted-Python] overview: new review queue venue

[Craig Rodrigues]

I submitted this PR, which is now closed:
https://github.com/twisted/twisted/pull/62

I don't want to re-open that PR, but I am using that as an example
As an example, if I wanted to re-open that, how would I go about doing it?

I am not an administrator of the Twisted GitHub project, so on that web
link,
there is no option for me to re-open the PR.

Are you suggesting that I would need to
   -> create a new branch in my repo with new commits
   -> create a new pull request

?

--
Craig

On Sun, May 22, 2016 at 9:39 PM, meejah  wrote:

>
> closing a PR with a nice note explaining, "please re-open when X is
> fixed/changed" scares away a potential contributer I have my doubts as
> to whether they would fix X if you *didn't* close it...)
>
>
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] overview: new review queue venue

[Amber "Hawkie" Brown]

Mark has been working on a bot which would reopen it with a comment: 
https://github.com/markrwilliams/txghbot

- Amber

> On 23 May 2016, at 12:56, Craig Rodrigues  wrote:
> 
> I submitted this PR, which is now closed: 
> https://github.com/twisted/twisted/pull/62
> 
> I don't want to re-open that PR, but I am using that as an example
> As an example, if I wanted to re-open that, how would I go about doing it?
> 
> I am not an administrator of the Twisted GitHub project, so on that web link,
> there is no option for me to re-open the PR.
> 
> Are you suggesting that I would need to
>-> create a new branch in my repo with new commits
>-> create a new pull request
> 
> ?
> 
> --
> Craig
> 
> On Sun, May 22, 2016 at 9:39 PM, meejah  wrote:
> 
> closing a PR with a nice note explaining, "please re-open when X is
> fixed/changed" scares away a potential contributer I have my doubts as
> to whether they would fix X if you *didn't* close it...)
> 
> ___
> Twisted-Python mailing list
> Twisted-Python@twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] overview: new review queue venue

[Tristan Seligmann]

Note that even without the bot, I believe you can just create a new PR for
the same branch, so it's not *too* bad, but definitely a little clunky.

On Mon, 23 May 2016 at 06:59 Amber "Hawkie" Brown 
wrote:

> Mark has been working on a bot which would reopen it with a comment:
> https://github.com/markrwilliams/txghbot
>
> - Amber
>
> > On 23 May 2016, at 12:56, Craig Rodrigues 
> wrote:
> >
> > I submitted this PR, which is now closed:
> https://github.com/twisted/twisted/pull/62
> >
> > I don't want to re-open that PR, but I am using that as an example
> > As an example, if I wanted to re-open that, how would I go about doing
> it?
> >
> > I am not an administrator of the Twisted GitHub project, so on that web
> link,
> > there is no option for me to re-open the PR.
> >
> > Are you suggesting that I would need to
> >-> create a new branch in my repo with new commits
> >-> create a new pull request
> >
> > ?
> >
> > --
> > Craig
> >
> > On Sun, May 22, 2016 at 9:39 PM, meejah  wrote:
> >
> > closing a PR with a nice note explaining, "please re-open when X is
> > fixed/changed" scares away a potential contributer I have my doubts as
> > to whether they would fix X if you *didn't* close it...)
> >
> > ___
> > Twisted-Python mailing list
> > Twisted-Python@twistedmatrix.com
> > http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
> ___
> Twisted-Python mailing list
> Twisted-Python@twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python