On 2013-05-10 08:57, Lars T. Kyllingstad wrote:

> This seems to require yet another tool besides Bugzilla and GitHub,
> which I think we should avoid.  It's bad enough to have information in
> two places, without scattering it even more.  If we can somehow make it
> work within the current toolset, then I am all for it.

We could use labels on github for this.

-- 
/Jacob Carlborg

On Fri, 10 May 2013 07:00:08 -0400, Jacob Carlborg <doob@me.com> wrote:

> On 2013-05-09 23:10, Steven Schveighoffer wrote:
>
>> OK, so now to implement this kind of thing, we need to have a
>> collaboration tool that allows tracking the review through its
>> workflow.  No idea what the best tool or what a good tool would be.  We
>> need something kind of like an issue tracker (can github issue tracker
>> do this?).  I don't have a lot of experience with doing online project
>> collaboration except with D.  I like trello, but it seems to have not
>> caught on here.
>
> Github can handle this. You can assign people to a pull request and add labels to indicate different statuses or whatever you want them to mean. At least I think you can add labels to pull requests. I don't currently have any open pull request for any of my projects so I cannot test it.
>

That sounds fine too.  I wasn't aware of that feature.

-Steve

On 2013-05-10 14:41, Steven Schveighoffer wrote:

> That sounds fine too.  I wasn't aware of that feature.

There are milestones as well, if that is of any use.

-- 
/Jacob Carlborg

On Fri, 10 May 2013 03:05:31 -0400, Daniel Murphy <yebblies@nospamgmail.com> wrote:

> "Steven Schveighoffer" <schveiguy@yahoo.com> wrote in message

>> 2. Make someone own the review.  Without ownership, there will be long
>> delays, and that reflects a sense of disapproval or apathy towards the
>> submitter or his idea, even if that isn't the case.  I like how the
>> "feature" review on the newsgroup has a review manager and he sets a
>> deadline.  That works out REALLY well.
>>
>
> My impression is that sometimes modules sit around for quite a while waiting
> for a review manager.  How are reviewers going to be assigned and how will
> it be ensured that they don't let if fall through the cracks?

We can't be perfect :)  This is a volunteer army.  If that is what happens, that is what happens.  At least we should make the submitter aware of that likelihood.

A weekly check of pull requests that are assigned but stale might be a good idea.

>> Oh!  I completely forgot!  Another requirement for a pull request to be
>> accepted:
>>
>>   g) Any fixed bugs MUST be accompanied by a unit test that fails with the
>> current code but passes with the change.
>>
>
> Sure, but this does not ensure that the tests have full coverage of the
> problem, or even full coverage of the cases presented in the bug report.
> This probably applies to dmd more than the others where failures can have
> very complex interacting causes.

Up to your discretion as a reviewer what you require.  It's impossible to formalize this requirement as a documented/objective measure.  I would say that 9 times out of 10, the bug report will contain a minimized example that causes the issue to occur, that should just be included as a unit-test.

>> If this is included, the automated tests should cover that.
>>
>
> I would love to have a way to add a test case, and have the autotester
> ensure both that it fails with the old version, and works with the new one.

I didn't consider that the auto-tester wouldn't test that the test case fails on the existing code.  I suppose that would just be up to the reviewer whether you wanted to go through the effort of testing with that test on the current master branch, or eyeball it and assume it does fail.  The most important thing is that a unit test is inserted to verify the fix actually works, and that a regression is caught immediately.

>> agreed.  I'm really leaning toward github issues since it already
>> integrates with github pull requests (right? never used it before),
>> probably involves less work to automate.
>>
>
> Yeah, I think we can use labels to set the states and it has assignment
> built in.

That is perfect.  A couple people have mentioned we can do this without github issues, the pull requests support labels and assignment.  Though mutually exclusive labels might be awkward.  Perhaps we should redefine the stages with that in mind.

>> I think we are on the same page, my stages don't have to be "real"
>> statuses, but it's important to formalize workflow and statuses so both
>> sides know what is going on and what is expected.
>
> I would like real statuses because they are searchable.  The wiki is almost
> good enough here, but having them connected to the actual pull requests and
> automatically changed when they fail the test suite would be much much
> better.  Maybe this applies better to dmd where the majority are bugfixes
> instead of enhancements, I don't know.  I think we're agreeing on most of
> it.

I meant not ALL the stages have to be real statuses :)  We clearly want something searchable to see the pull requests you are assigned and which ones are ready for review.

I will take a look at github issues and github's pull request features to see what level of features we need.

-Steve

"Steven Schveighoffer" <schveiguy@yahoo.com> wrote in message news:op.wwvee6wleav7ka@stevens-macbook-pro.local...
>> Sure, but this does not ensure that the tests have full coverage of the problem, or even full coverage of the cases presented in the bug report. This probably applies to dmd more than the others where failures can have very complex interacting causes.
>
> Up to your discretion as a reviewer what you require.  It's impossible to formalize this requirement as a documented/objective measure.  I would say that 9 times out of 10, the bug report will contain a minimized example that causes the issue to occur, that should just be included as a unit-test.
>

Ok, I think this might be different for dmd vs phobos.

>
> I didn't consider that the auto-tester wouldn't test that the test case fails on the existing code.  I suppose that would just be up to the reviewer whether you wanted to go through the effort of testing with that test on the current master branch, or eyeball it and assume it does fail. The most important thing is that a unit test is inserted to verify the fix actually works, and that a regression is caught immediately.
>

Absolutely agreed, this is fundamental.  After all it isn't really fixed unless you can guarantee it _stays_ fixed.  Adding tests also has the nice effect of pushing the burden of keeping it working onto the next person to modify the code. :)

>
> That is perfect.  A couple people have mentioned we can do this without github issues, the pull requests support labels and assignment.  Though mutually exclusive labels might be awkward.  Perhaps we should redefine the stages with that in mind.
>

They support labels and assignment because they automatically get an issue created for them.  I think you have to turn on issue support in order to actually add labels to the pull requests (or at least I can't find the interface).

On Friday, 10 May 2013 at 11:00:08 UTC, Jacob Carlborg wrote:
> Github can handle this. You can assign people to a pull request and add labels to indicate different statuses or whatever you want them to mean. At least I think you can add labels to pull requests. I don't currently have any open pull request for any of my projects so I cannot test it.

I believe only issues could have labels applied. Which is fine for those using issues since the pull will create an issue.

On 2013-05-10 20:10, Jesse Phillips wrote:

> I believe only issues could have labels applied. Which is fine for those
> using issues since the pull will create an issue.

But currently issues are disabled.

-- 
/Jacob Carlborg

Steven Schveighoffer wrote:
> OK, so now to implement this kind of thing, we need to have a collaboration tool that allows tracking the review through its workflow. No idea what the best tool or what a good tool would be.  We need something kind of like an issue tracker (can github issue tracker do this?).  I don't have a lot of experience with doing online project collaboration except with D.  I like trello, but it seems to have not caught on here.

The best tool for this is gerrit:

- http://en.wikipedia.org/wiki/Gerrit_%28software%29#Notable_users
- Presentation: http://skillsmatter.com/podcast/home/intro-git-gerrit
   I've downloaded and cut just the gerrit part:
   http://koch.ro/temp/gerrit_skillsmatter.mp4
- gerrit vs. github pull requests:
http://julien.danjou.info/blog/2013/rant-about-github-pull-request-workflow-implementation

Gerrit for D:

- Gerrit works on top of Git.
- Gerrit is designed to automatically run unit and style checks even before
a human reviews the code.
- Gerrit tracks the whole history of a change request across all versions of
this change. One can see diffs between versions of one particular change
request.
- Gerrit supports topics to further specify what a change is about

Regards, Thomas Koch