Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Wed, Jul 06, 2011 at 05:27:55PM -0400, Glyph Lefkowitz wrote: Looking at http://twistedmatrix.com/trac/ticket/3420 now, I see reviews with lots of functional issues and spec-compliance/correctness issues raised. There are a few notes on the API as well, but without addressing the reviews there, the patch would just have incorrect behavior. This is also my experience. When fixing issue 4378, I received mostly constructive feedback. There is much knowledge how to do things when working on the codebase, some of it implicit or spread over multiple documents. During review, this knowledge was conveyed in a tangible way, using a concrete piece of code. I have to admit that it was a little discouraging to receive a long list of change requests. But the reviewers did a very good job explaining why things should be changed. This helped me to adapt my patches to fulfill the standards for the codebase. And it was very satisfying to finally have the patch accepted :-). Best regards, -- Albert Brandl Weiermayer Solutions GmbH | Abteistraße 12, A-4813 Altmünster phone: +43 (0) 720 70 30 14| fax: +43 (0) 7612 20 3 56 web: http://www.weiermayer.com ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On 7/5/11 10:36 PM, Glyph Lefkowitz wrote: Can you point to a specific ticket where you think this was the case? I have this same general feeling, but pretty much all of the reviews I found when I went looking for specific examples included at least some significant coding-standard, documentation, and test coverage problems. If we can find more specific examples, perhaps we can prevent this from recurring. I was mostly thinking about the persistent connection functionality for twisted.web.client.Agent. Maybe Twisted would benefit more from having that functionality in place, than having the super-perfect API between Agent and HTTP parser. I do agree that we don't want to block every ticket on the absolute best possible implementation; but, allowing changes that don't have test and documentation coverage is a recipe for creating an unmaintainable mess. I agree. -- Johan Rydberg Product Designer Edgeware AB Mäster Samuelsgatan 56 SE-111 21 Stockholm, Sweden ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Wed, 2011-07-06 at 13:10 +0200, Johan Rydberg wrote: I was mostly thinking about the persistent connection functionality for twisted.web.client.Agent. We definitely want this to get in, this was a large part of the motivation for Agent in the first place. Maybe Twisted would benefit more from having that functionality in place, than having the super-perfect API between Agent and HTTP parser. The goal is not so much a perfect API as something we won't have to deprecate soon after because we realize there are some requirements that can't be addressed in a backwards compatible way. In this case, the cookie, proxy and other agent wrappers that have been created mean we now have a better understanding of what the Agent API looks like from a higher level, which should help. ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Jul 6, 2011, at 8:04 AM, Itamar Turner-Trauring wrote: On Wed, 2011-07-06 at 13:10 +0200, Johan Rydberg wrote: I was mostly thinking about the persistent connection functionality for twisted.web.client.Agent. We definitely want this to get in, this was a large part of the motivation for Agent in the first place. Looking at http://twistedmatrix.com/trac/ticket/3420 now, I see reviews with lots of functional issues and spec-compliance/correctness issues raised. There are a few notes on the API as well, but without addressing the reviews there, the patch would just have incorrect behavior. The few notes that are purely API aesthetics are mostly make this private by default, which should be a trivial search-and-replace to fix. Maybe Twisted would benefit more from having that functionality in place, than having the super-perfect API between Agent and HTTP parser. The goal is not so much a perfect API as something we won't have to deprecate soon after because we realize there are some requirements that can't be addressed in a backwards compatible way. In this case, the cookie, proxy and other agent wrappers that have been created mean we now have a better understanding of what the Agent API looks like from a higher level, which should help. That does sound a little like the perfect API to me :). And, I'd be inclined to argue - if I could find even one example of a functionally-correct patch with sufficient docs and tests that had actually been held up because of API issues :). -glyph ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Tue, Jul 5, 2011 at 7:36 AM, Johan Rydberg johan.rydb...@edgeware.tvwrote: On 7/1/11 6:08 PM, Itamar Turner-Trauring wrote: In order to have at least some anecdotal evidence -- I've had some patched rejected, probably on sound basis. But the experience always leave you with a feeling that you got stabbed. Yes, this is terrible. How can we fix it? Sometimes it _is_ be better to get some basic functionality in place, instead of arguing about how things should be done the right way. From time to time you find a ticket that you want fixed, and start working on it, but end up never submitting it since you already know that even if it fixes the problem it will be shot down, since it doesn't do it this or that way. Well, I think everyone would agree that a system where it's easier to pick up such a half-finished thing that just needs some love would be better, regardless of whether that unfinished work should go into twisted or not. Right? cheers lvh ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Jul 5, 2011, at 1:36 AM, Johan Rydberg wrote: On 7/1/11 6:08 PM, Itamar Turner-Trauring wrote: In order to have at least some anecdotal evidence -- I've had some patched rejected, probably on sound basis. But the experience always leave you with a feeling that you got stabbed. We're always very grateful for contributions. I'm very sad to hear that you feel like you got stabbed. Sometimes it _is_ be better to get some basic functionality in place, instead of arguing about how things should be done the right way. Can you point to a specific ticket where you think this was the case? I have this same general feeling, but pretty much all of the reviews I found when I went looking for specific examples included at least some significant coding-standard, documentation, and test coverage problems. If we can find more specific examples, perhaps we can prevent this from recurring. I do agree that we don't want to block every ticket on the absolute best possible implementation; but, allowing changes that don't have test and documentation coverage is a recipe for creating an unmaintainable mess. Trust me, having dealt with Twisted in both modes, it's harder to get a patch into a release if the requirement is demonstrate to everyone that it doesn't break everything without tests and documentation. It's just that it moves the work from you, the contributor, to a special hell that the release manager inhabits for months of painful pre-release debugging, or users who notice that all their applications are broken and file lots of bugs. From time to time you find a ticket that you want fixed, and start working on it, but end up never submitting it since you already know that even if it fixes the problem it will be shot down, since it doesn't do it this or that way. In these cases it would be best to file the ticket and describe your potential solution before implementing it. You can even put the 'review' keyword on it to indicate that you want feedback from a contributor before proceeding. -glyph ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On 7/1/11 6:08 PM, Itamar Turner-Trauring wrote: In order to have at least some anecdotal evidence -- I've had some patched rejected, probably on sound basis. But the experience always leave you with a feeling that you got stabbed. Sometimes it _is_ be better to get some basic functionality in place, instead of arguing about how things should be done the right way. From time to time you find a ticket that you want fixed, and start working on it, but end up never submitting it since you already know that even if it fixes the problem it will be shot down, since it doesn't do it this or that way. -- Johan Rydberg Product Designer Edgeware AB Mäster Samuelsgatan 56 SE-111 21 Stockholm, Sweden ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
[Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
In order to have at least some anecdotal evidence -- If you've submitted a patch to Twisted (or started a branch) and it never made it in, how did that happen? I imagine reasons might include a review request to write tests, redesign requests, getting distracted, it works for me, design discussions that never got anywhere... What happened in your case? (I'd also like someone -- lvh? -- to go through report 16 and see if we can come up with a summary of how those tickets ended up where they are.) ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On 01/07/11 17:08, Itamar Turner-Trauring wrote: In order to have at least some anecdotal evidence -- If you've submitted a patch to Twisted (or started a branch) and it never made it in, how did that happen? I imagine reasons might include a review request to write tests, redesign requests, getting distracted, it works for me, design discussions that never got anywhere... What happened in your case? I have submitted patches that went nowhere: http://twistedmatrix.com/trac/ticket/4602 http://twistedmatrix.com/trac/ticket/4603 http://twistedmatrix.com/trac/ticket/4610 http://twistedmatrix.com/trac/ticket/4666 Some of them seemed to be: No, don't do it this way, do it that way. I'll be honest, that's just completely demoralising, when you're contributing new functionality. On that topic, I don't think the Twisted process is as accessible as some of you guys think it is I'm afraid. I found the discussion about the IPv6 tickets a bit disheartening :o( However, more constructively (less whiney!) some tickets languished in make these tiny cleanups and that's just incredibly painful in the current setup, with SVN and Trac mediating things. I've got absolutely no interest in pulling SVN head, writing a patch, submitting it as an attachment via Trac and *then* being told ok, I've created this branch. Go off and learn how to do branches in a crappy old centralised VCS, and in a way compatible with UQDS, re-do your patch in a branch, then send another diff in as a file Really? I mean, come on guys... If it were git/github, then I'd simply make the incremental changes in my local git branch, re-push to github and re-submit the pull request. I hate Launchpad, but I'm sure it's equivalent beats the crap out of SVN. Please, for the love of god, adopt a DVCS which lets contributors develop continuously against a local branch and push changes! Cheers, Phil ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
Hi all, On 01.07.2011 18:36, Phil Mayers wrote: However, more constructively (less whiney!) some tickets languished in make these tiny cleanups and that's just incredibly painful in the current setup, with SVN and Trac mediating things. I've got absolutely no interest in pulling SVN head, writing a patch, submitting it as an attachment via Trac and *then* being told ok, I've created this branch. Go off and learn how to do branches in a crappy old centralised VCS, and in a way compatible with UQDS, re-do your patch in a branch, then send another diff in as a file I absolutely agree with Phil here. The twisted code and contribution standards are so high that patches from new contributors (like myself) are bound to be rejected/resubmitted at least once, maybe more. Don't get me wrong, I believe high standards are a good thing, but doing continuous development based on tools like svn and trac is really painful and it's really difficult to motivate yourself to work on a once rejected ticket. That being said, I believe that the move to a DVCS is a smart move for any project looking for continuous community contribution, because IMHO they simply allow for a more developer friendly process for all involved, thus making the whole review process a more friendly and less discouraging thing. Cheers, Chris ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Jul 1, 2011, at 1:08 PM, chris wrote: doing continuous development based on tools like svn and trac is really painful and it's really difficult to motivate yourself to work on a once rejected ticket Can you be more specific, please? What's painful? Procedurally, it's almost the same number of clicks (except for the unfortunate need to type the word 'review') to do this on Github or Launchpad. What part of the process is painful? If you're not a committer, we're not going to let you run code on our buildbots either way without a cursory review (that's just a recipe for automated attacks) so it's not like you get past that step for free, either. Plus, you can use the DVCS of your choice to actually author the patch. ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Jul 1, 2011, at 1:41 PM, Glyph Lefkowitz gl...@twistedmatrix.com wrote: On Jul 1, 2011, at 1:08 PM, chris wrote: doing continuous development based on tools like svn and trac is really painful and it's really difficult to motivate yourself to work on a once rejected ticket Can you be more specific, please? What's painful? I always found it especially irritating to come back to a patch later. If I don't already have a patched checkout of Twisted, I need to figure out what revision I was at before (or to actually be safe, make a HEAD checkout), then reapply my patch, hoping it is still valid. With a fork I can check it out any time, rebase to the current master (or branch I'm working on), having my changes reapplied for me. When I have made more changes I just push it up. No changes to tickets or switching keywords or watching Trac reject my patch file 10 times then clearing all my cookies or whatever. I've always admired Twisted's standards and process; I think they have made it possible for such a huge project to maintain working order for so long. The tools could use an upgrade, though. Procedurally, it's almost the same number of clicks (except for the unfortunate need to type the word 'review') to do this on Github or Launchpad. What part of the process is painful? If you're not a committer, we're not going to let you run code on our buildbots either way without a cursory review (that's just a recipe for automated attacks) so it's not like you get past that step for free, either. Plus, you can use the DVCS of your choice to actually author the patch. ___ 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
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
Sent via iPhone Is your email Premiere? On Jul 1, 2011, at 11:41, Glyph Lefkowitz gl...@twistedmatrix.com wrote: Can you be more specific, please? What's painful? Re-syncing whatever changes JP (just as an example of a reviewer) has made back into your local repo from SVN...which due to SVN's weakness on branch tracking makes merging fail frequently...then making your changes...generating a manual patch and finally uploading it to Trac. It would be far simpler to setup my DVCS to track JP's remote copy of my ticket's branch...then simply pull from that remote...make my changes and request he pull from me when he's ready to review. Automates the whole process quite a bit and reduces the round trip yak shaving. -J ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Fri, Jul 1, 2011 at 11:08 AM, Itamar Turner-Trauring ita...@itamarst.org wrote: In order to have at least some anecdotal evidence -- If you've submitted a patch to Twisted (or started a branch) and it never made it in, how did that happen? I imagine reasons might include a review request to write tests, redesign requests, getting distracted, it works for me, design discussions that never got anywhere... What happened in your case? (I'd also like someone -- lvh? -- to go through report 16 and see if we can come up with a summary of how those tickets ended up where they are.) I've had a few of these. I've also worked on others, reviewed a few, tried to resurrect some from abandonment, etc. Reasons include: - the finish line gets moved: A reviewer says do this and then it'll be good to go into trunk. The contributor does this and then another reviewer says oh just one more thing Rinse. Repeat. This is hugely demotivational. In many cases a better response from follow on reviewers would be to land the ticket, and then create another ticket for whatever other changes are needed. This is not always possible but I think it should be done where it is possible. - compatibility with some other unfinished ticket: The reviewer insists on compatibility with some other ticket, or waiting on some other ticket to land. This is not necessarily a bad thing, but if the other ticket takes 6 months to land...well, can we blame people for wandering off and/or forgetting about things? (to be fair, I think I've only seen this once, and I can't recall what it was or find the ticket at the moment). - I've seen a lot of tickets that die at the this needs tests phase. Requiring tests is a good thing, though. I think the only way to help this problem is for reviewers to provide more guidance as to how exactly to create those tests. Even figuring out where to put the tests is sometimes very difficult. Exarkun has been pretty good about this in the last year or so, IIRC. (It might be worth it to create a Trac keyword for this situation, maybe needs-tests. This would make it easy to find those tickets, which might be a decent entry point for certain types of new contributors) - Some people just seem to wander off. - I can think of at least one ticket where the author stopped using Windows, and it was a Windows ticket. - That same Windows ticket is still open, though I tried to revive it. It basically tries to do 2 things. One of them is pretty nice (makes the StdIO protocol work on windows), but it's not even clear what the other one is trying to do. So the ticket is in limbo until someone has the time to either figure it out or split the ticket in two. - I recall one ticket that got stalled, because noone was entirely sure who exactly had written the patch. There was a third party who had contributed it to Twisted. This was easily resolved with a google search and an email. Of course the ticket is still open because it needs tests... - One ticket was stalled because it required a merge forward. Which is not really obvious how to do, especially since the DIvmod site died. I'm sure there are dozens of other reasons, but these are cases I can recall off the top of my head. Kevin Horn ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Fri, Jul 1, 2011 at 12:41 PM, Glyph Lefkowitz gl...@twistedmatrix.comwrote: On Jul 1, 2011, at 1:08 PM, chris wrote: doing continuous development based on tools like svn and trac is really painful and it's really difficult to motivate yourself to work on a once rejected ticket Can you be more specific, please? What's painful? Procedurally, it's almost the same number of clicks (except for the unfortunate need to type the word 'review') to do this on Github or Launchpad. What part of the process is painful? If you're not a committer, we're not going to let you run code on our buildbots either way without a cursory review (that's just a recipe for automated attacks) so it's not like you get past that step for free, either. For me the pain isn't Trac, it's SVN. The more I use DVCS's the more I hate it. Also, Combinator does not work on Windows, and hasn't for years. And before you say submit patches, I did. They sat in the Divmod Trac instance for over a year, I requested reviews of the relevant tickets _daily_ for 3 months on Divmod's IRC channel, and they were never merged, or even reviewed, or even AFAIK put under version control. Now they're gone with the DIvmod site. If the tickets are ever recoverable, they were tickets #3001-#3004. Plus, you can use the DVCS of your choice to actually author the patch. In theory, yes, though it is not obvious how to do this in a way that is compatible with Twisted's workflow and Combinator. See http://twistedmatrix.com/trac/wiki/BazaarMirror for examples of some of the caveats of using DVCS on top of Subversion. Granted this works for just authoring a patch, which you then submit through Trac or whatever, but that doesn't really buy you a lot, IMO (though it does buy you something). Kevin Horn ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
It would be far simpler to setup my DVCS to track JP's remote copy of my ticket's branch...then simply pull from that remote...make my changes and request he pull from me when he's ready to review. Automates the whole process quite a bit and reduces the round trip yak shaving. Any reason you can't do this with our git or bzr mirrors? http://twistedmatrix.com/trac/wiki/BazaarMirror http://twistedmatrix.com/trac/wiki/GitMirror Or for that matter, you can include e.g. an github URL in the ticket instead of attaching the patch. ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Jul 1, 2011, at 2:06 PM, Tom Davis wrote: On Jul 1, 2011, at 1:41 PM, Glyph Lefkowitz gl...@twistedmatrix.com wrote: On Jul 1, 2011, at 1:08 PM, chris wrote: doing continuous development based on tools like svn and trac is really painful and it's really difficult to motivate yourself to work on a once rejected ticket Can you be more specific, please? What's painful? I always found it especially irritating to come back to a patch later. If I don't already have a patched checkout of Twisted, I need to figure out what revision I was at before (or to actually be safe, make a HEAD checkout), then reapply my patch, hoping it is still valid. With a fork I can check it out any time, rebase to the current master (or branch I'm working on), having my changes reapplied for me. When I have made more changes I just push it up. No changes to tickets or switching keywords or watching Trac reject my patch file 10 times then clearing all my cookies or whatever. Wow, does that actually happen? The rejecting the patchfile, I mean. That's terrible. In any case, that should not be important. You _should_ be able to use the DVCS of your choice to work on Twisted.[1] As I said in the previous message: Plus, you can use the DVCS of your choice to actually author the patch. Lots of people do submit patches using Git; I reviewed one earlier this week. One thing I think this thread has inspired is a prompt move to make it clearer how to do this. It would be great if you could help out lvh in producing some good copy for the various workflow-documentation pages so that it's clear to people how to use their favorite VCS if they already have one; the SVN-based diff-and-ptach instructions that are already there are meant for users who may not be familiar with _any_ kind of version control. (Just to forestall any objections: this is a realistic audience, lots of Twisted contributors have been in secondary school when they started.) I've always admired Twisted's standards and process; I think they have made it possible for such a huge project to maintain working order for so long. The tools could use an upgrade, though. Thanks for that :). And my earlier post notwithstanding, I agree. In fact, the tools have improved significantly, but many of them are things that contributors don't see (build system upgrades, release process improvements) but core developers, and therefore eventually users, do benefit from. -glyph [1]: Your choice, of course, should be Bazaar, as all other choices are wrong. But we can work around your Git habit ;-). ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Sat, Jul 2, 2011 at 1:27 AM, Glyph Lefkowitz gl...@twistedmatrix.comwrote: When you do a review, try to be as thorough as possible. Don't *ever* do a review that says update @since markers or 2 blank lines between methods and nothing else With Github's edit-this-file-on-the-web feature, it will effectively be easier to just fix it than it is to complain about how someone else needs to fix it. cheers lvh ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On 1 Jul, 11:27 pm, gl...@twistedmatrix.com wrote: On Jul 1, 2011, at 6:57 PM, David Ripton wrote: Working with patches because you don't have svn commit rights is annoying, but this annoyance is a relatively minor fixed cost. It's still important for us to reduce this cost; even if it's not the bottleneck, we have to optimize first where we can optimize :). The real issue, for controversial features, is achieving consensus, and then getting your feature in before consensus is lost. Yes, absolutely. And there's are some important guidelines for reviewers that can be inferred from this: Try to stick to coding-standard stuff as much as possible, especially if there's been a previous review. Don't bring up I think it would be better if... things, except to say file an additional ticket. We've disagreed about this in the past, so I don't think you'll be surprised if I say that I don't think this is a good idea. :) If an earlier review misses *functional* issues with a change, then they need to be brought up. Scope creep should be avoided at *all* stages of the process, but an incomplete first review doesn't exempt a change from the development requirements (and I don't think you think it should, even though it sounds like you're saying it here :). If there's a previous review, as much as possible, stick to the points brought up in the previous review. Make sure they're addressed, and try not to add a pile of conflicting stylistic suggestions. Stylistic issues should all be known in advance (read the coding standard, etc) and brought up in the first review (because the first reviewer should know them too). Stylistic issues that aren't covered by the coding standard definitely shouldn't be sprung in a subsequent review (or the reviewer should address them himself or herself) - or even a first review, really. This is a separate case from pointing out *functional* issues that the first review missed. When you do a review, try to be as thorough as possible. Don't ever do a review that says update @since markers or 2 blank lines between methods and nothing else; at least, you need to say ... and then it will be ready to merge. Remember that when you take it out of review, no other reviewer is going to look at it until the author fixes it and resubmits it, which may be quite a while. If you feel like adding some partial commentary to help the next full reviewer, just add a comment, don't remove the review keyword. This is very important, since it should reduce the instances in which a later review does have to introduce a new point. I don't think anyone benefits from forgoing resolving functional issues that are detected after the first review but before the change actually lands. Be explicit about what happens next, even if it's going to be redundant. Always say ... and it will need another review or ... and then merge. Try not to voice a vague dissatisfaction with the architecture of something without an explicit suggestion about (A) what should be done, and (B) whether it needs to be done before the feature can be merged. For contributors, one suggestion: make implementation details private as much as possible, so that the reviewer will have to consider the aesthetics of the implementation details less. The smaller the public API of the contribution, the easier it is to avoid bikeshedding around method names and class placement :). There are plenty of issues on the contribution-accepting side which I don't want to minimize, but I think there's another thing contributors can do to help the overall process. If a review results in more work than you're interested in doing, say so. Make it clear that you're no longer taking responsibility for the ticket. Then there's some chance that another contributor might take it the rest of the way (without waiting 5 years before deciding the original contributor has lost interest). None of this would have helped in particular on the IPv6 stuff, but given that that affected an extremely core API, and had a ton of fiddly little details, I'm not sure much could have helped on that one... Sooo fiddly aaghhgh. I know I've broken these rules myself on occasion, and I'd like to encourage other reviewers to call me out on it when they notice it :). This raises another point, which is that the mailing list isn't a terribly useful place for these points to end up. If anything is actually expected to change, then the need to update the review documentation (such as it is) and probably also get serious about meta- reviews. Jean-Paul ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Fri, Jul 01, 2011 at 09:11:34PM +0200, Laurens Van Houtven wrote: On Fri, Jul 1, 2011 at 8:59 PM, Itamar Turner-Trauring ita...@itamarst.orgwrote: Or for that matter, you can include e.g. an github URL in the ticket instead of attaching the patch. Only if there's a decent Github mirror to fork from, otherwise you're asking people to do a multi-hour operation (I know, because I'm doing it right now) to get a decent git repo, Last time I tried (perhaps a year ago), a git-svn clone of the Twisted SVN repo took the better part of a week. I seem to recall somebody preparing a tarball of a git-svn-clone'd repository to help people bootstrap, but my clone was already completed at that point so I didn't investigate further. ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Sat, Jul 2, 2011 at 02:23, Tim Allen screwt...@froup.com wrote: On Fri, Jul 01, 2011 at 09:11:34PM +0200, Laurens Van Houtven wrote: Only if there's a decent Github mirror to fork from, otherwise you're asking people to do a multi-hour operation (I know, because I'm doing it right now) to get a decent git repo, Last time I tried (perhaps a year ago), a git-svn clone of the Twisted SVN repo took the better part of a week. I seem to recall somebody preparing a tarball of a git-svn-clone'd repository to help people bootstrap, but my clone was already completed at that point so I didn't investigate further. I update the tarball a few times a year at http://ludios.net/mirror/ - see Twisted-checkout-README.txt for notes. If you do it yourself, keep in mind that git svn clone has to restart at r1 several times, for reasons I don't fully understand (due to partial SVN branches?). A few months back, a branch created by bzr with an svn:mergeinfo property caused it to restart at r1 again. This adds about 27 hours to the git svn clone time, unfortunately. Ivan ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] SURVEY: Have you submitted a patch to Twisted and it never got in?
On Fri, Jul 1, 2011 at 6:01 PM, Jason J. W. Williams jasonjwwilli...@gmail.com wrote: Because they don't always seem to track the ticket branch folders in a timely manner. Especially, when JP (he seems to usually be my reviewer :) ) pushes a modification of my patch to the ticket branch. It's at this point trying to merge in from SVN is usually a nightmare. Why it is a nightmare? Just do svn checkout of the ticket branch and continue your work and submit additional patches against it if needed. The only problem here that I could see is if you have made some changes in addition to your patch. But in this case kdiff3 makes it a snap to merge you changes to the ticket branch checkout. My Git copy being tied to an older SVN rev that my patch is based on. SVN just seems to lose it's brains when my patch isn't in the SVN commit history, because SVN repo doesn't allow me to commit in. I can't decipher this, could you elaborate? DVCS would allow me to branch, commit to my repo, and then let JP pull from my repo into his to review and push up to the Twisted repo when he's happy with it...and all of the commit history is sane from the original, to my patch to his changes, so when I go to pull back down from the Twisted repo everything merges sanely. IMHO the common practice is to accept patches for review and potential inclusion and pull only from a trusted lieutenants (like in Linux kernel case) and creating patches is not very different in svn, git etc. Regards, Mikhail Terekhov ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
