Quantcast

ZF2 Pull Request usage - WIP Pull Requests

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta (Ocramius)
Hello there,

Today I've been discussing a bit with Arthur Bodera (Thinkscape) on IRC about how to play around with branches and pull requests. At some point I suggested Arthur to open a Pull Request on ZF2 master with his new CLI refactoring (which isn't finished nor ready for merging yet).
What came out is that the CR team has to close PR that are not ready for merging or really near to it.

So here I am probably asking something already discussed, but I'd like to ask it anyway: are Work In Progress (WIP) Pull Requests allowed?
I'd love to see that.
It is currently really difficult to track changes to the framework without asking who is refactoring what, in which repository and on which branch. I'm not asking for a PR management that tracks refactoring from the beginning, I'm asking for some earlier opened PRs, so that the community can see what is going on and follow the discussion directly on the code.

For instance, take
https://github.com/weierophinney/zf2/commits/feature/view-layer . That branch contains features that are being reworked since the 12th of January. From my point of view, a PR could have been opened around https://github.com/weierophinney/zf2/commit/895846709b5ad1135919f2de1e994d68723c65a1 (18 days ago).

Same applies for other WIP/refactorings.

The main idea behind this proposal is that I, as well as the guys on IRC and those following the mailing list, do know WHERE to look for changes, but the entire community doesn't. Also, the community cannot follow the discussion around refactoring/coding that is going on. Github is a powerful tool and could help a lot in this development process.
By opening a pull request before completion of coding/refactoring, the community (all of it, including visitors, curious people and eventually very good coders who could really be helpful with just some feedback!) can track changes without following parallel discussion threads on the mailing list/IRC.

I'd just suggest to any core contributors to open PRs earlier and allow the entire community to help while still coding.
What do you think about that?

P.S.: Not using my standard email account as the ML doesn't like my posts :(
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

weierophinney
Administrator
-- Marco Pivetta <[hidden email]> wrote
(on Monday, 20 February 2012, 10:05 PM +0100):

> Today I've been discussing a bit with Arthur Bodera (Thinkscape) on
> IRC about how to play around with branches and pull requests. At some
> point I suggested Arthur to open a Pull Request on ZF2 master with his
> new CLI refactoring (which isn't finished nor ready for merging yet).
>
> What came out is that the CR team has to close PR that are not ready
> for merging or really near to it.
>
> So here I am probably asking something already discussed, but I'd like
> to ask it anyway: are Work In Progress (WIP) Pull Requests allowed?
> I'd love to see that.  It is currently really difficult to track
> changes to the framework without asking who is refactoring what, in
> which repository and on which branch.

There's actually two places you can find this information currently.

 * The first, linked in the ZF2 subsite, is our Kanban board:
 
    http://framework.zend.com/zf2/board

   The dropdown at the top allows you to filter by beta target, as well
   as a variety of other tags (such as "mvc", "db", "cli", etc.).
   Typically, we will comment in a story as to what repo and branch
   contains the WIP.

   The board has only recently been exposed, though, so not all
   information is captured well here.

 * Evan Coury maintains a tool that tracks recent activity on known
   contributor's branches:
 
    http://zf2.evan.pro/recent

   This is an excellent visualization for seeing what components are
   actively being worked on, by whom, and where.

   I'll be linking this into the ZF2 subsite's sidebar this week.

> I'm not asking for a PR management that tracks refactoring from the
> beginning, I'm asking for some earlier opened PRs, so that the
> community can see what is going on and follow the discussion directly
> on the code.
>
> For instance, take >
> https://github.com/weierophinney/zf2/commits/feature/view-layer . That
> branch > contains features that are being reworked since the 12th of
> January. From my > point of view, a PR could have been opened around
> https://github.com/ >
> weierophinney/zf2/commit/895846709b5ad1135919f2de1e994d68723c65a1 (18
> days ago).
>
> Same applies for other WIP/refactorings.

We have our current PR policy for a variety of reasons.

The primary one, however, is that due to the way PRs are listed by
GitHub, it's easiest if we assume that all PRs in that queue have the
expectation that they can be reviewed for merge immediately. Why?
Because if it's marked as a "WIP", how are we going to know when it's
ready? Due to how notifications are done on GitHub, I may or may not
receive an email when a comment is made on the PR -- which means I then
need to manually go through each PR that remains in the queue, read
through comments, and determine if action is necessary.

It's hard enough already when a PR is made and we determine that it's
not ready to merge due to missing tests or documentation; the policy I'm
defaulting to is that if I need the author to clean up anything, I close
the request and tell them to open a new one; it simply makes running
through the PR queue simpler.

> The main idea behind this proposal is that I, as well as the guys on
> IRC and those following the mailing list, do know WHERE to look for
> changes, but the entire community doesn't. Also, the community cannot
> follow the discussion around refactoring/coding that is going on.
> Github is a powerful tool and could help a lot in this development
> process.
>
> By opening a pull request before completion of coding/refactoring, the
> community (all of it, including visitors, curious people and
> eventually very good coders who could really be helpful with just some
> feedback!) can track changes without following parallel discussion
> threads on the mailing list/IRC.

I'd argue in cases like this, we (speaking as a contributor) should be
posting to the list detailing (a) what we're working on, and (b) where
you can review the code. (I've tried to do this for the view layer,
though I realize now that I haven't updated the list since over a week
before I issued the PR for it.)

I feel that this approach, combined with the tools I linked above, should
provide the necessary insight developers need in order to review
incoming changes.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta (Ocramius)
This post was updated on .
 I perfectly understand your point and I'm also aware of the different
tracking tools that have been put at the disposal of the community, but if
it wasn't me telling people I chat with that there was some work going on
on the view layer, well, then those people would never have known...

I think the WIP status is up to the PR author, not to the reviewer...
Having an issue marked as [WIP] is enough to have it skipped while
reviewing imo. I don't see where the additional overhead resides, except of
having a prefix for the PR title. I know you are the one usually working on
the merges, and that this idea obviously is a weight on your shoulders, but
I don't see why you should be the one deciding if the PR is to be merged. I
personally tend to explicitly write if it is a WIP (and for now I have
never seen a PR merged while explicitly being marked WIP).

Here's the problem: Github is a great tool, but it is limited.
In fact, there is no decent way of building a discussion thread based on a
branch of a certain repository if not on a pull request (which tracks that
branch). If we wanted to discuss any of these great works going on on
repositories of Thinkscape, DASPRiD, Ezimuel, Ralphschindler and yours,
well, we would be forced to get out of github, having information
fragmented over different medias (IRC/ML/Issue tracker/Blog posts), which
is what currently happens.
That is surely not optimal, and also if people like EvanDotPro are building
cool and useful tools to try and keep things together, I currently don't
see anything better than a pull request.

Also, I've been already asked a couple of times if development on ZF2 is
stalling, and had to explain why it seems that there's not much activity on
the repository and that zf2 is not following the WIP PR method.

I'm probably influenced by Doctrine 2 ORM, but taking a look at
https://github.com/doctrine/doctrine2/pulls you can see there's good stuff,
bad stuff and lots of good ideas, comments and commits deriving from those
comments. For me, pull request != merge request. Pull request is a
discussion thread, merging things is not the important point of the thing.
Most of the discussions brought new ideas or even got some influence on the
ORM direction... I like that way, and I see no real problem in editing PR
titles and adding [WIP] to stall them.


So here's what I got for now:

Cons:

  1. Overhead caused by longer list of PRs
  2. Probably stalled pull requests hanging on for months (see doctrine2)

Pros:

  1. Current development branches are tracked and visible to
  "community-unaware" visitors, the community is better informed about what's
  moving and where. Eventually, we could receive CR from users who just
  browse for fun (I do it sometimes! Without PRs that wouldn't be possible!)
  2. Code could generate discussion, discussion could generate code, in a
  single thread (which is the coolest feature of the pull request), with
  (probably) better review
  3. Github integration (because users are lazy, I'm a particularly lazy
  user :P )

So here's my idea: can we get 5-10 minutes of the discussion of tomorrow (I
really hope I can join that) to discuss this and see what opinions are? :)

2012/2/21 Matthew Weier O'Phinney <matthew@zend.com>

> -- Marco Pivetta <info@marco-pivetta.com> wrote
> (on Monday, 20 February 2012, 10:05 PM +0100):
> > Today I've been discussing a bit with Arthur Bodera (Thinkscape) on
> > IRC about how to play around with branches and pull requests. At some
> > point I suggested Arthur to open a Pull Request on ZF2 master with his
> > new CLI refactoring (which isn't finished nor ready for merging yet).
> >
> > What came out is that the CR team has to close PR that are not ready
> > for merging or really near to it.
> >
> > So here I am probably asking something already discussed, but I'd like
> > to ask it anyway: are Work In Progress (WIP) Pull Requests allowed?
> > I'd love to see that.  It is currently really difficult to track
> > changes to the framework without asking who is refactoring what, in
> > which repository and on which branch.
>
> There's actually two places you can find this information currently.
>
>  * The first, linked in the ZF2 subsite, is our Kanban board:
>
>    http://framework.zend.com/zf2/board
>
>   The dropdown at the top allows you to filter by beta target, as well
>   as a variety of other tags (such as "mvc", "db", "cli", etc.).
>   Typically, we will comment in a story as to what repo and branch
>   contains the WIP.
>
>   The board has only recently been exposed, though, so not all
>   information is captured well here.
>
>  * Evan Coury maintains a tool that tracks recent activity on known
>   contributor's branches:
>
>    http://zf2.evan.pro/recent
>
>   This is an excellent visualization for seeing what components are
>   actively being worked on, by whom, and where.
>
>   I'll be linking this into the ZF2 subsite's sidebar this week.
>
> > I'm not asking for a PR management that tracks refactoring from the
> > beginning, I'm asking for some earlier opened PRs, so that the
> > community can see what is going on and follow the discussion directly
> > on the code.
> >
> > For instance, take >
> > https://github.com/weierophinney/zf2/commits/feature/view-layer . That
> > branch > contains features that are being reworked since the 12th of
> > January. From my > point of view, a PR could have been opened around
> > https://github.com/ >
> > weierophinney/zf2/commit/895846709b5ad1135919f2de1e994d68723c65a1 (18
> > days ago).
> >
> > Same applies for other WIP/refactorings.
>
> We have our current PR policy for a variety of reasons.
>
> The primary one, however, is that due to the way PRs are listed by
> GitHub, it's easiest if we assume that all PRs in that queue have the
> expectation that they can be reviewed for merge immediately. Why?
> Because if it's marked as a "WIP", how are we going to know when it's
> ready? Due to how notifications are done on GitHub, I may or may not
> receive an email when a comment is made on the PR -- which means I then
> need to manually go through each PR that remains in the queue, read
> through comments, and determine if action is necessary.
>
> It's hard enough already when a PR is made and we determine that it's
> not ready to merge due to missing tests or documentation; the policy I'm
> defaulting to is that if I need the author to clean up anything, I close
> the request and tell them to open a new one; it simply makes running
> through the PR queue simpler.
>
> > The main idea behind this proposal is that I, as well as the guys on
> > IRC and those following the mailing list, do know WHERE to look for
> > changes, but the entire community doesn't. Also, the community cannot
> > follow the discussion around refactoring/coding that is going on.
> > Github is a powerful tool and could help a lot in this development
> > process.
> >
> > By opening a pull request before completion of coding/refactoring, the
> > community (all of it, including visitors, curious people and
> > eventually very good coders who could really be helpful with just some
> > feedback!) can track changes without following parallel discussion
> > threads on the mailing list/IRC.
>
> I'd argue in cases like this, we (speaking as a contributor) should be
> posting to the list detailing (a) what we're working on, and (b) where
> you can review the code. (I've tried to do this for the view layer,
> though I realize now that I haven't updated the list since over a week
> before I issued the PR for it.)
>
> I feel that this approach, combined with the tools I linked above, should
> provide the necessary insight developers need in order to review
> incoming changes.
>
> --
> Matthew Weier O'Phinney
> Project Lead            | matthew@zend.com
> Zend Framework          | http://framework.zend.com/
> PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Artur Bodera
On Tue, Feb 21, 2012 at 9:55 AM, Marco Pivetta <[hidden email]> wrote:
So here's my idea: can we get 5-10 minutes of the discussion of tomorrow (I really hope I can join that) to discuss this and see what opinions are? :)

Bring it on!

Add to agenda...

ps: are there any open-source projects that use github in this way? If so, I'd propose we take a look at how they manage the info. flow and if it's any good. Also - there's always a third way :-)




-- 
      __
     /.)\   +48 695 600 936
     \(./   [hidden email]




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Jurian Sluiman
In reply to this post by weierophinney
Hi Matthew,

2012/2/21 Matthew Weier O'Phinney <[hidden email]>
We have our current PR policy for a variety of reasons.

The primary one, however, is that due to the way PRs are listed by
GitHub, it's easiest if we assume that all PRs in that queue have the
expectation that they can be reviewed for merge immediately. Why?
Because if it's marked as a "WIP", how are we going to know when it's
ready? Due to how notifications are done on GitHub, I may or may not
receive an email when a comment is made on the PR -- which means I then
need to manually go through each PR that remains in the queue, read
through comments, and determine if action is necessary.

GitHub is able to label pull requests with self-defined labels. It might be an option to label the "ready" PRs as ready-to-merge or the opposite: the WiP PRs as work-in-progress. I'd argue the latter is preferable.

Mostly people will PR a finished codebase with the intend to merge it directly. Only when something is missing, not good enough etcetera the PR will be closed. A much smaller amount of contributors have the intend to PR a WiP. Therefore I'd suggest such PRs (like Cli and perhaps other long-term refactoring components like Form, Log, Db, Navigation) to be opened and marked with a special label. This gives the opportunity to discuss the development while it's easy for others to track such changes.

This means the policy remains intact for most PRs, as those will be hotfixes (especially after the zf2.0 stable release). Some PRs can be marked as WiP, can be opened with unfinished code and remain open until they are ready to merge. Because those WiP are larger parts of zf2, it is likely there is enough communication (irc, mail) when the label can be removed. To keep the number of WiP PRs small, you might even get the CR team involved if it is OK to open a WiP PR for a specific change. The moment the Zend or CR team reviews PRs to be merged, the only thing to bear in mind is to skip the PRs with that specific label.
--
Jurian Sluiman
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Alex Major
On Tue, Feb 21, 2012 at 10:59 AM, Jurian Sluiman <[hidden email]> wrote:
Hi Matthew,

2012/2/21 Matthew Weier O'Phinney <[hidden email]>
We have our current PR policy for a variety of reasons.

The primary one, however, is that due to the way PRs are listed by
GitHub, it's easiest if we assume that all PRs in that queue have the
expectation that they can be reviewed for merge immediately. Why?
Because if it's marked as a "WIP", how are we going to know when it's
ready? Due to how notifications are done on GitHub, I may or may not
receive an email when a comment is made on the PR -- which means I then
need to manually go through each PR that remains in the queue, read
through comments, and determine if action is necessary.

GitHub is able to label pull requests with self-defined labels. It might be an option to label the "ready" PRs as ready-to-merge or the opposite: the WiP PRs as work-in-progress. I'd argue the latter is preferable.

Mostly people will PR a finished codebase with the intend to merge it directly. Only when something is missing, not good enough etcetera the PR will be closed. A much smaller amount of contributors have the intend to PR a WiP. Therefore I'd suggest such PRs (like Cli and perhaps other long-term refactoring components like Form, Log, Db, Navigation) to be opened and marked with a special label. This gives the opportunity to discuss the development while it's easy for others to track such changes.

This means the policy remains intact for most PRs, as those will be hotfixes (especially after the zf2.0 stable release). Some PRs can be marked as WiP, can be opened with unfinished code and remain open until they are ready to merge. Because those WiP are larger parts of zf2, it is likely there is enough communication (irc, mail) when the label can be removed. To keep the number of WiP PRs small, you might even get the CR team involved if it is OK to open a WiP PR for a specific change. The moment the Zend or CR team reviews PRs to be merged, the only thing to bear in mind is to skip the PRs with that specific label.
--
Jurian Sluiman

Opening a pull request whilst as a WIP seems like the right idea as long as it's correctly labelled. Internally we create feature branches whilst working on new components or major refactoring but that doesn't work with open source projects.

The other advantage is that code review can be a gradual piece if the pull request is opened whilst it's still being developed. Githubs inline comments are great for giving detailed feedback and having that review during development is much simpler (or at a minimum, quicker) than doing it once 'complete'.

Just my $0.02.

Alex.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

akrabat
In reply to this post by Jurian Sluiman

On 21 Feb 2012, at 10:59, Jurian Sluiman wrote:

> GitHub is able to label pull requests with self-defined labels. It might be an option to label the "ready" PRs as ready-to-merge or the opposite: the WiP PRs as work-in-progress. I'd argue the latter is preferable.
>

I can't see that feature. I can see that you can add a label to an issue, but not to a PR. Could you point me at it?


> Mostly people will PR a finished codebase with the intend to merge it directly. Only when something is missing, not good enough etcetera the PR will be closed. A much smaller amount of contributors have the intend to PR a WiP. Therefore I'd suggest such PRs (like Cli and perhaps other long-term refactoring components like Form, Log, Db, Navigation) to be opened and marked with a special label. This gives the opportunity to discuss the development while it's easy for others to track such changes.

In my opinion, a single thread isn't a good way to handle many conversations about a significant refactor / new feature. e.g. Artur wants to discuss console routing, whilst someone else is discussing, say, use of exceptions within console and they are both in the same single thread of discussion.

I personally see PRs as "This code is ready to be merged unless you find something wrong" and should be short lived. A lot of open PRs looks like a management problem and risks us missing good contributions within the noise of long-lived PRs that may never get merged.

I do wonder if the desire to use PRs to track work in progress come about because it's quite hard in github to find the correct diff for a given branch. i.e. for Matthew's view-layer, you need https://github.com/weierophinney/zf2/compare/zendframework:master...feature/view-layer which is what I've been using throughout the development to keep up with his changes.  We should automate this link on Evan's tool really.


> This means the policy remains intact for most PRs, as those will be hotfixes (especially after the zf2.0 stable release). Some PRs can be marked as WiP, can be opened with unfinished code and remain open until they are ready to merge. Because those WiP are larger parts of zf2, it is likely there is enough communication (irc, mail) when the label can be removed. To keep the number of WiP PRs small, you might even get the CR team involved if it is OK to open a WiP PR for a specific change. The moment the Zend or CR team reviews PRs to be merged, the only thing to bear in mind is to skip the PRs with that specific label.

Who would get to decide that a given PR is 'blessed' with being allowed to hang around for months? I don't like the idea of a value judgement being used for this as it would be in danger of resulting with a clique having the right to create WIP PRs and consequentially suggest to new contributors that their ideas are somehow less important.

I would also argue that the larger feature branches are the last ones you want as PRs during dev as the design may change a lot based on feedback and as I mentioned above, a single thread of comments is very hard to keep track of when discussing design decisions and enhancement ideas that may go nowhere.  PRs for completed code tend to only have specific comments on code issues that need fixing and are, as a result, much more focussed.



Regards,

Rob...
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta
This post has NOT been accepted by the mailing list yet.
Rob, still, a single thread is better than no thread...

Marco Pivetta

http://twitter.com/Ocramius     

http://marco-pivetta.com    



On 21 February 2012 13:59, akrabat [via Zend Framework Community] <[hidden email]> wrote:

On 21 Feb 2012, at 10:59, Jurian Sluiman wrote:

> GitHub is able to label pull requests with self-defined labels. It might be an option to label the "ready" PRs as ready-to-merge or the opposite: the WiP PRs as work-in-progress. I'd argue the latter is preferable.
>

I can't see that feature. I can see that you can add a label to an issue, but not to a PR. Could you point me at it?


> Mostly people will PR a finished codebase with the intend to merge it directly. Only when something is missing, not good enough etcetera the PR will be closed. A much smaller amount of contributors have the intend to PR a WiP. Therefore I'd suggest such PRs (like Cli and perhaps other long-term refactoring components like Form, Log, Db, Navigation) to be opened and marked with a special label. This gives the opportunity to discuss the development while it's easy for others to track such changes.

In my opinion, a single thread isn't a good way to handle many conversations about a significant refactor / new feature. e.g. Artur wants to discuss console routing, whilst someone else is discussing, say, use of exceptions within console and they are both in the same single thread of discussion.

I personally see PRs as "This code is ready to be merged unless you find something wrong" and should be short lived. A lot of open PRs looks like a management problem and risks us missing good contributions within the noise of long-lived PRs that may never get merged.

I do wonder if the desire to use PRs to track work in progress come about because it's quite hard in github to find the correct diff for a given branch. i.e. for Matthew's view-layer, you need https://github.com/weierophinney/zf2/compare/zendframework:master...feature/view-layer which is what I've been using throughout the development to keep up with his changes.  We should automate this link on Evan's tool really.


> This means the policy remains intact for most PRs, as those will be hotfixes (especially after the zf2.0 stable release). Some PRs can be marked as WiP, can be opened with unfinished code and remain open until they are ready to merge. Because those WiP are larger parts of zf2, it is likely there is enough communication (irc, mail) when the label can be removed. To keep the number of WiP PRs small, you might even get the CR team involved if it is OK to open a WiP PR for a specific change. The moment the Zend or CR team reviews PRs to be merged, the only thing to bear in mind is to skip the PRs with that specific label.

Who would get to decide that a given PR is 'blessed' with being allowed to hang around for months? I don't like the idea of a value judgement being used for this as it would be in danger of resulting with a clique having the right to create WIP PRs and consequentially suggest to new contributors that their ideas are somehow less important.

I would also argue that the larger feature branches are the last ones you want as PRs during dev as the design may change a lot based on feedback and as I mentioned above, a single thread of comments is very hard to keep track of when discussing design decisions and enhancement ideas that may go nowhere.  PRs for completed code tend to only have specific comments on code issues that need fixing and are, as a result, much more focussed.



Regards,

Rob...


If you reply to this email, your message will be added to the discussion below:
http://zend-framework-community.634137.n4.nabble.com/ZF2-Pull-Request-usage-WIP-Pull-Requests-tp4405109p4406935.html
To start a new topic under ZF Contributor, email [hidden email]
To unsubscribe from ZF Contributor, click here.
NAML

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta (Ocramius)
This post has NOT been accepted by the mailing list yet.
In reply to this post by akrabat
Rob, still, a single thread is better than no thread... Or dozens of unlinked threads.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Tomáš Fejfar
In reply to this post by akrabat
So what about a forum (SMF is the best to my taste)? It's easy to manage. It's easy to create categories for current work (say Zend\Form) and have distinct threads underneath it. It's much more managable than the mailing list IMO. It's also ready to "divert" offtopic discussion to other threads. There are also extensive tools to create polls, that may be useful to quickly settle a smaller debate. It's also insanely simple to "pin" the hot discussions to the top of the list - so that everyone see it. It's also easier to read/search for normal users then Nabble. 

Last but not least - it's super easy to see what changed since last visit. 

Even though I don't have much time now, I'd be happy to help moderating, as I have some previous skills in this area :)

I really like the fact that in ML you can settle a personal debate privately by removing zf-c from CC, but overall UX of the ML is awful IMO. 

But.... (to sum up) I can't see how a single threaded conversation below PR can replace neither ML nor a forum. It would just split the focus IMO. 

On Tue, Feb 21, 2012 at 1:59 PM, Rob Allen <[hidden email]> wrote:

On 21 Feb 2012, at 10:59, Jurian Sluiman wrote:

> GitHub is able to label pull requests with self-defined labels. It might be an option to label the "ready" PRs as ready-to-merge or the opposite: the WiP PRs as work-in-progress. I'd argue the latter is preferable.
>

I can't see that feature. I can see that you can add a label to an issue, but not to a PR. Could you point me at it?


> Mostly people will PR a finished codebase with the intend to merge it directly. Only when something is missing, not good enough etcetera the PR will be closed. A much smaller amount of contributors have the intend to PR a WiP. Therefore I'd suggest such PRs (like Cli and perhaps other long-term refactoring components like Form, Log, Db, Navigation) to be opened and marked with a special label. This gives the opportunity to discuss the development while it's easy for others to track such changes.

In my opinion, a single thread isn't a good way to handle many conversations about a significant refactor / new feature. e.g. Artur wants to discuss console routing, whilst someone else is discussing, say, use of exceptions within console and they are both in the same single thread of discussion.

I personally see PRs as "This code is ready to be merged unless you find something wrong" and should be short lived. A lot of open PRs looks like a management problem and risks us missing good contributions within the noise of long-lived PRs that may never get merged.

I do wonder if the desire to use PRs to track work in progress come about because it's quite hard in github to find the correct diff for a given branch. i.e. for Matthew's view-layer, you need https://github.com/weierophinney/zf2/compare/zendframework:master...feature/view-layer which is what I've been using throughout the development to keep up with his changes.  We should automate this link on Evan's tool really.


> This means the policy remains intact for most PRs, as those will be hotfixes (especially after the zf2.0 stable release). Some PRs can be marked as WiP, can be opened with unfinished code and remain open until they are ready to merge. Because those WiP are larger parts of zf2, it is likely there is enough communication (irc, mail) when the label can be removed. To keep the number of WiP PRs small, you might even get the CR team involved if it is OK to open a WiP PR for a specific change. The moment the Zend or CR team reviews PRs to be merged, the only thing to bear in mind is to skip the PRs with that specific label.

Who would get to decide that a given PR is 'blessed' with being allowed to hang around for months? I don't like the idea of a value judgement being used for this as it would be in danger of resulting with a clique having the right to create WIP PRs and consequentially suggest to new contributors that their ideas are somehow less important.

I would also argue that the larger feature branches are the last ones you want as PRs during dev as the design may change a lot based on feedback and as I mentioned above, a single thread of comments is very hard to keep track of when discussing design decisions and enhancement ideas that may go nowhere.  PRs for completed code tend to only have specific comments on code issues that need fixing and are, as a result, much more focussed.



Regards,

Rob...

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta
It is not about replacing, it is about gaining more entry point for the community to contribute.
I see no harm in opening a PR, discuss coding that is going on and reference discussions and chat logs from there...
It's like a lighthouse placed in the middle of the project, it tells new visitors where to go and what to look for (because they don't visit "zend-framework-community.634137.n4.nabble.com" (srsly?)) nor IRC (because I myself use an own private server as a tunnel, and not everyone does...).
I personally also think that a list of open PR makes things easier to follow.

Marco Pivetta

http://twitter.com/Ocramius     

http://marco-pivetta.com    



2012/2/21 Tomáš Fejfar <[hidden email]>
So what about a forum (SMF is the best to my taste)? It's easy to manage. It's easy to create categories for current work (say Zend\Form) and have distinct threads underneath it. It's much more managable than the mailing list IMO. It's also ready to "divert" offtopic discussion to other threads. There are also extensive tools to create polls, that may be useful to quickly settle a smaller debate. It's also insanely simple to "pin" the hot discussions to the top of the list - so that everyone see it. It's also easier to read/search for normal users then Nabble. 

Last but not least - it's super easy to see what changed since last visit. 

Even though I don't have much time now, I'd be happy to help moderating, as I have some previous skills in this area :)

I really like the fact that in ML you can settle a personal debate privately by removing zf-c from CC, but overall UX of the ML is awful IMO. 

But.... (to sum up) I can't see how a single threaded conversation below PR can replace neither ML nor a forum. It would just split the focus IMO. 


On Tue, Feb 21, 2012 at 1:59 PM, Rob Allen <[hidden email]> wrote:

On 21 Feb 2012, at 10:59, Jurian Sluiman wrote:

> GitHub is able to label pull requests with self-defined labels. It might be an option to label the "ready" PRs as ready-to-merge or the opposite: the WiP PRs as work-in-progress. I'd argue the latter is preferable.
>

I can't see that feature. I can see that you can add a label to an issue, but not to a PR. Could you point me at it?


> Mostly people will PR a finished codebase with the intend to merge it directly. Only when something is missing, not good enough etcetera the PR will be closed. A much smaller amount of contributors have the intend to PR a WiP. Therefore I'd suggest such PRs (like Cli and perhaps other long-term refactoring components like Form, Log, Db, Navigation) to be opened and marked with a special label. This gives the opportunity to discuss the development while it's easy for others to track such changes.

In my opinion, a single thread isn't a good way to handle many conversations about a significant refactor / new feature. e.g. Artur wants to discuss console routing, whilst someone else is discussing, say, use of exceptions within console and they are both in the same single thread of discussion.

I personally see PRs as "This code is ready to be merged unless you find something wrong" and should be short lived. A lot of open PRs looks like a management problem and risks us missing good contributions within the noise of long-lived PRs that may never get merged.

I do wonder if the desire to use PRs to track work in progress come about because it's quite hard in github to find the correct diff for a given branch. i.e. for Matthew's view-layer, you need https://github.com/weierophinney/zf2/compare/zendframework:master...feature/view-layer which is what I've been using throughout the development to keep up with his changes.  We should automate this link on Evan's tool really.


> This means the policy remains intact for most PRs, as those will be hotfixes (especially after the zf2.0 stable release). Some PRs can be marked as WiP, can be opened with unfinished code and remain open until they are ready to merge. Because those WiP are larger parts of zf2, it is likely there is enough communication (irc, mail) when the label can be removed. To keep the number of WiP PRs small, you might even get the CR team involved if it is OK to open a WiP PR for a specific change. The moment the Zend or CR team reviews PRs to be merged, the only thing to bear in mind is to skip the PRs with that specific label.

Who would get to decide that a given PR is 'blessed' with being allowed to hang around for months? I don't like the idea of a value judgement being used for this as it would be in danger of resulting with a clique having the right to create WIP PRs and consequentially suggest to new contributors that their ideas are somehow less important.

I would also argue that the larger feature branches are the last ones you want as PRs during dev as the design may change a lot based on feedback and as I mentioned above, a single thread of comments is very hard to keep track of when discussing design decisions and enhancement ideas that may go nowhere.  PRs for completed code tend to only have specific comments on code issues that need fixing and are, as a result, much more focussed.



Regards,

Rob...


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

H Glenn Hatfield
In reply to this post by Artur Bodera

On Feb 21, 2012, at 3:37 AM, Artur Bodera wrote:

> ps: are there any open-source projects that use github in this way? If so, I'd propose we take a look at how they manage the info. flow and if it's any good. Also - there's always a third way :-)

Github lists 3 "popular open source projects" at https://github.com/features/projects/codereview (Sinatra, Homebrew and Rails) which all seem to use pull requests a little differently. I did not see any specific WiP labels, but it does appear that Homebrew and Rails have tags that indicate something is ready for review.

To Rob's question about applying labels to pull requests, it's done on the Issues page. Each pull request creates an issue and the api can be used to create a pull request for an already existing issue.

-H
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

weierophinney
Administrator
-- H Glenn Hatfield <[hidden email]> wrote
(on Tuesday, 21 February 2012, 08:29 AM -0700):

> On Feb 21, 2012, at 3:37 AM, Artur Bodera wrote:
> > ps: are there any open-source projects that use github in this way?
> > If so, I'd propose we take a look at how they manage the info. flow
> > and if it's any good. Also - there's always a third way :-)
>
> Github lists 3 "popular open source projects" at
> https://github.com/features/projects/codereview (Sinatra, Homebrew and
> Rails) which all seem to use pull requests a little differently. I did
> not see any specific WiP labels, but it does appear that Homebrew and
> Rails have tags that indicate something is ready for review.
>
> To Rob's question about applying labels to pull requests, it's done on
> the Issues page. Each pull request creates an issue and the api can be
> used to create a pull request for an already existing issue.

... which doesn't work for us, as we've disabled the GH issue tracker in
order to use something more full-featured.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta (Ocramius)
@Glenn: title = "[WIP] " + title is enough imo... I used the doctrine example before, and they don't use Github's issue tracker either...

2012/2/21 Matthew Weier O'Phinney <[hidden email]>
-- H Glenn Hatfield <[hidden email]> wrote
(on Tuesday, 21 February 2012, 08:29 AM -0700):
> On Feb 21, 2012, at 3:37 AM, Artur Bodera wrote:
> > ps: are there any open-source projects that use github in this way?
> > If so, I'd propose we take a look at how they manage the info. flow
> > and if it's any good. Also - there's always a third way :-)
>
> Github lists 3 "popular open source projects" at
> https://github.com/features/projects/codereview (Sinatra, Homebrew and
> Rails) which all seem to use pull requests a little differently. I did
> not see any specific WiP labels, but it does appear that Homebrew and
> Rails have tags that indicate something is ready for review.
>
> To Rob's question about applying labels to pull requests, it's done on
> the Issues page. Each pull request creates an issue and the api can be
> used to create a pull request for an already existing issue.

... which doesn't work for us, as we've disabled the GH issue tracker in
order to use something more full-featured.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta
In reply to this post by weierophinney
 Hi there!
This post is mainly to restart the discussion after what has been talked about in today's weekly IRC meeting.
The idea of keeping WIP pull requests open right now by just letting PRs open has been discarded, because adding [WIP] in the title is not enough to recognize them and filter them.
The target of this entire discussion is to increase ZF2 visibility to allow receiving more code reviews and comments on what is going on.
Some ideas would make it possible to make the ZF2 repository shine a bit more on Github:

Another thing that could be done is adding readme files also in components/skeleton directories to link official documentation or give useful hints to the developers.

What do you think? For me, the README.txt => README.md transition with links to relevant articles/docs/development is a must, and I see no problem in doing so. I would ask about the tagging feature. Would it be too chaotic to have the issue tracker? Should we ask the guys at github?

Marco Pivetta

http://twitter.com/Ocramius   


On 21 February 2012 18:09, Matthew Weier O'Phinney <[hidden email]> wrote:
-- H Glenn Hatfield <[hidden email]> wrote
(on Tuesday, 21 February 2012, 08:29 AM -0700):
> On Feb 21, 2012, at 3:37 AM, Artur Bodera wrote:
> > ps: are there any open-source projects that use github in this way?
> > If so, I'd propose we take a look at how they manage the info. flow
> > and if it's any good. Also - there's always a third way :-)
>
> Github lists 3 "popular open source projects" at
> https://github.com/features/projects/codereview (Sinatra, Homebrew and
> Rails) which all seem to use pull requests a little differently. I did
> not see any specific WiP labels, but it does appear that Homebrew and
> Rails have tags that indicate something is ready for review.
>
> To Rob's question about applying labels to pull requests, it's done on
> the Issues page. Each pull request creates an issue and the api can be
> used to create a pull request for an already existing issue.

... which doesn't work for us, as we've disabled the GH issue tracker in
order to use something more full-featured.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Marco Pivetta (Ocramius)
In reply to this post by weierophinney
 Hi there!
This post is mainly to restart the discussion after what has been talked about in today's weekly IRC meeting.
The idea of keeping WIP pull requests open right now by just letting PRs open has been discarded, because adding [WIP] in the title is not enough to recognize them and filter them.
The target of this entire discussion is to increase ZF2 visibility to allow receiving more code reviews and comments on what is going on.
Some ideas would make it possible to make the ZF2 repository shine a bit more on Github:

Another thing that could be done is adding readme files also in components/skeleton directories to link official documentation or give useful hints to the developers.

What do you think? For me, the README.txt => README.md transition with links to relevant articles/docs/development is a must, and I see no problem in doing so. I would ask about the tagging feature. Would it be too chaotic to have the issue tracker? Should we ask the guys at github?

Marco Pivetta

http://twitter.com/Ocramius   

On 21 February 2012 18:09, Matthew Weier O'Phinney <[hidden email]> wrote:
-- H Glenn Hatfield <[hidden email]> wrote
(on Tuesday, 21 February 2012, 08:29 AM -0700):
> On Feb 21, 2012, at 3:37 AM, Artur Bodera wrote:
> > ps: are there any open-source projects that use github in this way?
> > If so, I'd propose we take a look at how they manage the info. flow
> > and if it's any good. Also - there's always a third way :-)
>
> Github lists 3 "popular open source projects" at
> https://github.com/features/projects/codereview (Sinatra, Homebrew and
> Rails) which all seem to use pull requests a little differently. I did
> not see any specific WiP labels, but it does appear that Homebrew and
> Rails have tags that indicate something is ready for review.
>
> To Rob's question about applying labels to pull requests, it's done on
> the Issues page. Each pull request creates an issue and the api can be
> used to create a pull request for an already existing issue.

... which doesn't work for us, as we've disabled the GH issue tracker in
order to use something more full-featured.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ZF2 Pull Request usage - WIP Pull Requests

Tomáš Fejfar
I'm not sure how github¨s flow fits into ZF¨s workflow. The issue tracker seems quite extensive (https://github.com/symfony/symfony/issues?labels=BC+Break&sort=created&direction=desc&state=open&page=1) whereas the pull requests are less complex. I guess the "correct" workflow for GitHub would be: 
Create issue marked as improvement. 
Insert PullRequests for completed parts of the feature to the thread of that issue (as seen in https://github.com/symfony/symfony/issues/2605) rather than creating pullrequests for each feature request. 

To sum up  it¨s obvious that tight github integration would help collaboration and contribution to ZF. On the other hand the "free/cool" workflow of github won't match very well with kind of rigid (in a good way) development that ZF has. I also think, that it's ad idea to split issues to two places. For example doctrine uses JIRA for issues. 

I noticed the "issue overflow" that is on JIRA. Average issue age is nearly TWO YEARS! Some of them are outdated, open and marked as blocker living happily since 2010 (http://framework.zend.com/issues/browse/ZFINC-79). I can't imagine what would happen to GitHub issues this way. 

Maybe it's not the tool that we use, but something in the process that is rotten :)

On Wed, Mar 7, 2012 at 10:44 PM, Marco Pivetta <[hidden email]> wrote:
 Hi there!
This post is mainly to restart the discussion after what has been talked about in today's weekly IRC meeting.
The idea of keeping WIP pull requests open right now by just letting PRs open has been discarded, because adding [WIP] in the title is not enough to recognize them and filter them.
The target of this entire discussion is to increase ZF2 visibility to allow receiving more code reviews and comments on what is going on.
Some ideas would make it possible to make the ZF2 repository shine a bit more on Github:

Another thing that could be done is adding readme files also in components/skeleton directories to link official documentation or give useful hints to the developers.

What do you think? For me, the README.txt => README.md transition with links to relevant articles/docs/development is a must, and I see no problem in doing so. I would ask about the tagging feature. Would it be too chaotic to have the issue tracker? Should we ask the guys at github?

Marco Pivetta

http://twitter.com/Ocramius   

On 21 February 2012 18:09, Matthew Weier O'Phinney <[hidden email]> wrote:
-- H Glenn Hatfield <[hidden email]> wrote
(on Tuesday, 21 February 2012, 08:29 AM -0700):
> On Feb 21, 2012, at 3:37 AM, Artur Bodera wrote:
> > ps: are there any open-source projects that use github in this way?
> > If so, I'd propose we take a look at how they manage the info. flow
> > and if it's any good. Also - there's always a third way :-)
>
> Github lists 3 "popular open source projects" at
> https://github.com/features/projects/codereview (Sinatra, Homebrew and
> Rails) which all seem to use pull requests a little differently. I did
> not see any specific WiP labels, but it does appear that Homebrew and
> Rails have tags that indicate something is ready for review.
>
> To Rob's question about applying labels to pull requests, it's done on
> the Issues page. Each pull request creates an issue and the api can be
> used to create a pull request for an already existing issue.

... which doesn't work for us, as we've disabled the GH issue tracker in
order to use something more full-featured.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc


Loading...