[Bug Tracker] Issues that need to be reviewed
Locked
[Bug Tracker] Issues that need to be reviewed
 
|
Re: [Bug Tracker] Issues that need to be reviewed
|
|
I systematically mark my fixes as ReviewNeeded because a second pair
of eyes don't hurt. But this seems to be yet another bottleneck. Taking responsibility to review might seem scary and involving especially when changes touch obscure/complex part of the system. So maybe the review could take lighter, more or less automated forms: - does automated integration reports a regression ? - does the bug report gives enough technical solution details or a good rationale ? - are there some tests provided ? Maybe you could handle a confidence score per developer. with a priori and a posteriori informations. Like "good solution, but the integrator had more work to finish the job"... On the other hand, Pharo claims the right to do some mistakes (and soon correct them), so maybe this review phase could be relaxed a bit. And one thing I still ask for is a diff log for each SLICE posted in the inbox, entirely browsable from the issue tracker or from the bug list, because to review you need: - to read issue tracker report - AND to inspect the changes with Monticello merge in a recent clean image... Having the diff would be a great economy for the reviewers. Or maybe this already happened? Maybe the report generated by Monticello that pops up once the package commited, could contain such diff that we would manually copy/past in the bug entry. Would that be cheap enough ? Nicolas Le 24 février 2012 08:30, Marcus Denker <[hidden email]> a écrit : > > http://code.google.com/p/pharo/issues/list?can=2&q=status%3AFixReviewNeeded > > -- > Marcus Denker -- http://marcusdenker.de > > |
Re: [Bug Tracker] Issues that need to be reviewed
|
|
On Feb 24, 2012, at 1:35 PM, Nicolas Cellier wrote:
> I systematically mark my fixes as ReviewNeeded because a second pair > of eyes don't hurt. > But this seems to be yet another bottleneck. > Taking responsibility to review might seem scary and involving > especially when changes touch obscure/complex part of the system. > > So maybe the review could take lighter, more or less automated forms: > - does automated integration reports a regression ? > - does the bug report gives enough technical solution details or a > good rationale ? > - are there some tests provided ? > > Maybe you could handle a confidence score per developer. with a priori > and a posteriori informations. > Like "good solution, but the integrator had more work to finish the job"... > > On the other hand, Pharo claims the right to do some mistakes (and > soon correct them), so maybe this review phase could be relaxed a bit. > > And one thing I still ask for is a diff log for each SLICE posted in > the inbox, entirely browsable from the issue tracker or from the bug > list, because to review you need: > - to read issue tracker report > - AND to inspect the changes with Monticello merge in a recent clean image... > Having the diff would be a great economy for the reviewers. > Or maybe this already happened? > > Maybe the report generated by Monticello that pops up once the package > commited, could contain such diff that we would manually copy/past in > the bug entry. Would that be cheap enough ? > > Nicolas +999. I don't mind reviewing changes, but loading up an image, updating it to latest version to ensure any conflicts are caught, opening MC browser, finding the slice, diffing against image, and only THEN be able to do a decent review are just too many manual steps, other than for issues I care very strongly about. (in which case I'm often the one requesting ReviewNeeded to begin with…) IMHO, doing all the above, and reporting merge conflicts/ new failed tests/a diff to the issue would be a perfect task for Ulyssé :) (As it is now, "Monkey went bananas" doesn't really tell me anything) Cheers, Henry |
Re: [Bug Tracker] Issues that need to be reviewed
|
|
I agree that everything (that could be done automatically) should be automated (By Ulysse or whatever), but you need
some human interaction to see if the change is good enough (coherent, consistency, design flaws, etc.).
As a newbie i'm always looking for someone to review my easy fixes. :) On Fri, Feb 24, 2012 at 10:17 AM, Henrik Johansen <[hidden email]> wrote:
|
Re: [Bug Tracker] Issues that need to be reviewed
|
|
On Feb 24, 2012, at 3:08 PM, Bernardo Ezequiel Contreras wrote: > I agree that everything (that could be done automatically) should be automated (By Ulysse or whatever), but you need > some human interaction to see if the change is good enough (coherent, consistency, design flaws, etc.). > As a newbie i'm always looking for someone to review my easy fixes. :) > Absolutely, what I wrote was intended to be about how to facilitate that human interaction by improving the parts of that process which are automatable, and in fact so cumbersome that the barrier to actually reviewing is in most cases too high. I doubt you would find anyone arguing a review can be fully automated with satisfying results. :) Cheers, Henry |
Re: [Bug Tracker] Issues that need to be reviewed
|
|
In reply to this post by Henrik Sperre Johansen
Le 24 février 2012 14:17, Henrik Johansen
<[hidden email]> a écrit : > On Feb 24, 2012, at 1:35 PM, Nicolas Cellier wrote: > >> I systematically mark my fixes as ReviewNeeded because a second pair >> of eyes don't hurt. >> But this seems to be yet another bottleneck. >> Taking responsibility to review might seem scary and involving >> especially when changes touch obscure/complex part of the system. >> >> So maybe the review could take lighter, more or less automated forms: >> - does automated integration reports a regression ? >> - does the bug report gives enough technical solution details or a >> good rationale ? >> - are there some tests provided ? >> >> Maybe you could handle a confidence score per developer. with a priori >> and a posteriori informations. >> Like "good solution, but the integrator had more work to finish the job"... >> >> On the other hand, Pharo claims the right to do some mistakes (and >> soon correct them), so maybe this review phase could be relaxed a bit. >> >> And one thing I still ask for is a diff log for each SLICE posted in >> the inbox, entirely browsable from the issue tracker or from the bug >> list, because to review you need: >> - to read issue tracker report >> - AND to inspect the changes with Monticello merge in a recent clean image... >> Having the diff would be a great economy for the reviewers. >> Or maybe this already happened? >> >> Maybe the report generated by Monticello that pops up once the package >> commited, could contain such diff that we would manually copy/past in >> the bug entry. Would that be cheap enough ? >> >> Nicolas > > +999. > > I don't mind reviewing changes, but loading up an image, updating it to latest version to ensure any conflicts are caught, opening MC browser, finding the slice, diffing against image, and only THEN be able to do a decent review are just too many manual steps, other than for issues I care very strongly about. (in which case I'm often the one requesting ReviewNeeded to begin with…) > > IMHO, doing all the above, and reporting merge conflicts/ new failed tests/a diff to the issue would be a perfect task for Ulyssé :) > (As it is now, "Monkey went bananas" doesn't really tell me anything) > > Cheers, > Henry Obviously, when you see there is a regression, test previously green (blue) and now red, you want to see which SLICE was loaded and in deeper details what changed in code if you want to quickly identify and fix the cause... How do you diff source code between two Jenkins build currently? Ideally such information should be accessible directly from the Jenkins report... Nicolas |
Re: [Bug Tracker] Issues that need to be reviewed
|
|
In reply to this post by Nicolas Cellier
On Feb 24, 2012, at 1:35 PM, Nicolas Cellier wrote: > I systematically mark my fixes as ReviewNeeded because a second pair > of eyes don't hurt. Yes I love to read other code. - just in this moment I have too many jobs (second of lab, head of team, pushing company…) > But this seems to be yet another bottleneck. > Taking responsibility to review might seem scary and involving > especially when changes touch obscure/complex part of the system. > > So maybe the review could take lighter, more or less automated forms: > - does automated integration reports a regression ? > - does the bug report gives enough technical solution details or a > good rationale ? > - are there some tests provided ? Indeed. > Maybe you could handle a confidence score per developer. with a priori > and a posteriori informations. > Like "good solution, but the integrator had more work to finish the job"… Yes :) > On the other hand, Pharo claims the right to do some mistakes (and > soon correct them), so maybe this review phase could be relaxed a bit. It is. We want to totally change the process We want to use Ulyss to automatically load code and report > And one thing I still ask for is a diff log for each SLICE posted in > the inbox, entirely browsable from the issue tracker or from the bug > list, because to review you need: > - to read issue tracker report > - AND to inspect the changes with Monticello merge in a recent clean image... > Having the diff would be a great economy for the reviewers. > Or maybe this already happened? the problem is that squeak source suffered. But yes this would be great. > Maybe the report generated by Monticello that pops up once the package > commited, could contain such diff that we would manually copy/past in > the bug entry. Would that be cheap enough ? > > Nicolas > > Le 24 février 2012 08:30, Marcus Denker <[hidden email]> a écrit : >> >> http://code.google.com/p/pharo/issues/list?can=2&q=status%3AFixReviewNeeded >> >> -- >> Marcus Denker -- http://marcusdenker.de >> >> > |
Re: [Bug Tracker] Issues that need to be reviewed
|
|
In reply to this post by Henrik Sperre Johansen
Henrik yes we are aware of that.
Stef On Feb 24, 2012, at 2:17 PM, Henrik Johansen wrote: > On Feb 24, 2012, at 1:35 PM, Nicolas Cellier wrote: > >> I systematically mark my fixes as ReviewNeeded because a second pair >> of eyes don't hurt. >> But this seems to be yet another bottleneck. >> Taking responsibility to review might seem scary and involving >> especially when changes touch obscure/complex part of the system. >> >> So maybe the review could take lighter, more or less automated forms: >> - does automated integration reports a regression ? >> - does the bug report gives enough technical solution details or a >> good rationale ? >> - are there some tests provided ? >> >> Maybe you could handle a confidence score per developer. with a priori >> and a posteriori informations. >> Like "good solution, but the integrator had more work to finish the job"... >> >> On the other hand, Pharo claims the right to do some mistakes (and >> soon correct them), so maybe this review phase could be relaxed a bit. >> >> And one thing I still ask for is a diff log for each SLICE posted in >> the inbox, entirely browsable from the issue tracker or from the bug >> list, because to review you need: >> - to read issue tracker report >> - AND to inspect the changes with Monticello merge in a recent clean image... >> Having the diff would be a great economy for the reviewers. >> Or maybe this already happened? >> >> Maybe the report generated by Monticello that pops up once the package >> commited, could contain such diff that we would manually copy/past in >> the bug entry. Would that be cheap enough ? >> >> Nicolas > > +999. > > I don't mind reviewing changes, but loading up an image, updating it to latest version to ensure any conflicts are caught, opening MC browser, finding the slice, diffing against image, and only THEN be able to do a decent review are just too many manual steps, other than for issues I care very strongly about. (in which case I'm often the one requesting ReviewNeeded to begin with…) > > IMHO, doing all the above, and reporting merge conflicts/ new failed tests/a diff to the issue would be a perfect task for Ulyssé :) > (As it is now, "Monkey went bananas" doesn't really tell me anything) > > Cheers, > Henry |
«
Return to Pharo Smalltalk Developers
|
1 view|%1 views
| Free forum by Nabble | Edit this page |