Code review tools?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
35 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Code review tools?

Igor Stasenko
Hello,

here is some thoughts what i would like to have.. one day.

imagine guy A did some change(s) to code, and guy B wants to review it.

Currently we having only one tool which helps a little: MC diff viewer..
but main tool is still human..
And it is quite tedious for guy B to go somewhere, copy pieces of code
and/or comment on things..

what would be really cool to have is to integrate some steroids into
diff viewer where guy B can press "create a review report"
and write his comments directly under selected change .. then post it
somewhere on web (or send emal),
so guy A can see directly which of his changes made guy B unhappy.

Going further, and integrating this into pharo development process, as
one guy (stef) said:
automation is good, but no changes should be made into image unless at
least one human took a look at it.

So, i thinking if we could have more streamlined review process, this
could help a bit (or a lot)..
because personally, for me it is tedious today, because integrator or
reviewer has to do many things manually,
which is not fun and takes too much precious time :)

I imagining something like opening a window, pressing "get list of
pending reviews from bug tracker"
and then selecting a change, review it and pressing "approve or
disapprove" which then will trigger the rest of automatic
things like integration into image and/or putting entry into bug
tracker with reviewer comments.

--
Best regards,
Igor Stasenko.

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Sean P. DeNigris
Administrator
Igor Stasenko wrote
what would be really cool to have is to integrate some steroids into
diff viewer where guy B can press "create a review report"
and write his comments directly under selected change .. then post it
somewhere on web (or send emal),
so guy A can see directly which of his changes made guy B unhappy.
This would be very cool. For me, this is one of github's killer features. Dale and I were having conversations about particular statements right in the diff for the commit. What makes it even more crazy is that there is markup to hyperlink to issues/users/etc
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Dennis Schetinin
In reply to this post by Igor Stasenko
It is a good idea for GSoC.


--

Best regards,


Dennis Schetinin



2013/3/11 Igor Stasenko <[hidden email]>
Hello,

here is some thoughts what i would like to have.. one day.

imagine guy A did some change(s) to code, and guy B wants to review it.

Currently we having only one tool which helps a little: MC diff viewer..
but main tool is still human..
And it is quite tedious for guy B to go somewhere, copy pieces of code
and/or comment on things..

what would be really cool to have is to integrate some steroids into
diff viewer where guy B can press "create a review report"
and write his comments directly under selected change .. then post it
somewhere on web (or send emal),
so guy A can see directly which of his changes made guy B unhappy.

Going further, and integrating this into pharo development process, as
one guy (stef) said:
automation is good, but no changes should be made into image unless at
least one human took a look at it.

So, i thinking if we could have more streamlined review process, this
could help a bit (or a lot)..
because personally, for me it is tedious today, because integrator or
reviewer has to do many things manually,
which is not fun and takes too much precious time :)

I imagining something like opening a window, pressing "get list of
pending reviews from bug tracker"
and then selecting a change, review it and pressing "approve or
disapprove" which then will trigger the rest of automatic
things like integration into image and/or putting entry into bug
tracker with reviewer comments.

--
Best regards,
Igor Stasenko.


Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Frank Shearar-3
In reply to this post by Sean P. DeNigris
On 11 March 2013 02:19, Sean P. DeNigris <[hidden email]> wrote:

> Igor Stasenko wrote
>> what would be really cool to have is to integrate some steroids into
>> diff viewer where guy B can press "create a review report"
>> and write his comments directly under selected change .. then post it
>> somewhere on web (or send emal),
>> so guy A can see directly which of his changes made guy B unhappy.
>
> This would be very cool. For me, this is one of github's killer features.
> Dale and I were having conversations about particular statements right in
> the diff for the commit. What makes it even more crazy is that there is
> markup to hyperlink to issues/users/etc

It's probably less work to get tools working with github than it is to
re-invent an in-image github.

frank

> Sean

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Camillo Bruni-3
On 2013-03-11, at 09:09, Frank Shearar <[hidden email]> wrote:

> On 11 March 2013 02:19, Sean P. DeNigris <[hidden email]> wrote:
>> Igor Stasenko wrote
>>> what would be really cool to have is to integrate some steroids into
>>> diff viewer where guy B can press "create a review report"
>>> and write his comments directly under selected change .. then post it
>>> somewhere on web (or send emal),
>>> so guy A can see directly which of his changes made guy B unhappy.
>>
>> This would be very cool. For me, this is one of github's killer features.
>> Dale and I were having conversations about particular statements right in
>> the diff for the commit. What makes it even more crazy is that there is
>> markup to hyperlink to issues/users/etc
>
> It's probably less work to get tools working with github than it is to
> re-invent an in-image github.

there is torch http://soft.vub.ac.be/torch/Home.html by Verónica Uquillas Gómez
which I presume does most of the things you want. AFAIK it doesn't run on 2.0
yet, maybe I am wrong ;).

Besides that we have Erwann doing and internship here working on validation of
Configurations, which is also going into the same direction.

The we have CodeCritics for validating code, which BTW is already in the image.
Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Igor Stasenko
In reply to this post by Frank Shearar-3
On 11 March 2013 09:09, Frank Shearar <[hidden email]> wrote:

> On 11 March 2013 02:19, Sean P. DeNigris <[hidden email]> wrote:
>> Igor Stasenko wrote
>>> what would be really cool to have is to integrate some steroids into
>>> diff viewer where guy B can press "create a review report"
>>> and write his comments directly under selected change .. then post it
>>> somewhere on web (or send emal),
>>> so guy A can see directly which of his changes made guy B unhappy.
>>
>> This would be very cool. For me, this is one of github's killer features.
>> Dale and I were having conversations about particular statements right in
>> the diff for the commit. What makes it even more crazy is that there is
>> markup to hyperlink to issues/users/etc
>
> It's probably less work to get tools working with github than it is to
> re-invent an in-image github.
>
i am very happy that github exists.. and its features available only if you
put code there..
but if code is not on github, then what you will do?

> frank
>
>> Sean
>



--
Best regards,
Igor Stasenko.

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Frank Shearar-3
On 11 March 2013 13:34, Igor Stasenko <[hidden email]> wrote:

> On 11 March 2013 09:09, Frank Shearar <[hidden email]> wrote:
>> On 11 March 2013 02:19, Sean P. DeNigris <[hidden email]> wrote:
>>> Igor Stasenko wrote
>>>> what would be really cool to have is to integrate some steroids into
>>>> diff viewer where guy B can press "create a review report"
>>>> and write his comments directly under selected change .. then post it
>>>> somewhere on web (or send emal),
>>>> so guy A can see directly which of his changes made guy B unhappy.
>>>
>>> This would be very cool. For me, this is one of github's killer features.
>>> Dale and I were having conversations about particular statements right in
>>> the diff for the commit. What makes it even more crazy is that there is
>>> markup to hyperlink to issues/users/etc
>>
>> It's probably less work to get tools working with github than it is to
>> re-invent an in-image github.
>>
> i am very happy that github exists.. and its features available only if you
> put code there..
> but if code is not on github, then what you will do?

To rephrase the question: what do we still need to do before we can
integrate (something like) github into a standard Smalltalk workflow?

frank

>> frank
>>
>>> Sean
>>
>
>
>
> --
> Best regards,
> Igor Stasenko.
>

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Esteban A. Maringolo
2013/3/11 Frank Shearar <[hidden email]>
>
> On 11 March 2013 13:34, Igor Stasenko <[hidden email]> wrote:
> > i am very happy that github exists.. and its features available only if you
> > put code there..
> > but if code is not on github, then what you will do?
>
> To rephrase the question: what do we still need to do before we can
> integrate (something like) github into a standard Smalltalk workflow?

I totally agree on that. But Github is great if your project is open
source or you're willing to pay for private repos.
On the other hand, BitBucket.org has free private repositories. But it
doesn't have code review.

In my case it is not a required feature, and I'd go for the already
implemented Github alternative.

Every time I see smalltalkers forking away from established mainstream
solutions/practices I hear a big noise inside my head. On the
opposite, whenever I see stuff like the CI integrations, FileTree,
standards compliance, and so on, I'm really happy.

Regards,

Esteban.

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

stephane ducasse

On Mar 11, 2013, at 4:05 PM, Esteban A. Maringolo <[hidden email]> wrote:

> 2013/3/11 Frank Shearar <[hidden email]>
>>
>> On 11 March 2013 13:34, Igor Stasenko <[hidden email]> wrote:
>>> i am very happy that github exists.. and its features available only if you
>>> put code there..
>>> but if code is not on github, then what you will do?
>>
>> To rephrase the question: what do we still need to do before we can
>> integrate (something like) github into a standard Smalltalk workflow?
>
> I totally agree on that. But Github is great if your project is open
> source or you're willing to pay for private repos.
> On the other hand, BitBucket.org has free private repositories. But it
> doesn't have code review.
>
> In my case it is not a required feature, and I'd go for the already
> implemented Github alternative.
>
> Every time I see smalltalkers forking away from established mainstream
> solutions/practices I hear a big noise inside my head. On the
> opposite, whenever I see stuff like the CI integrations, FileTree,
> standards compliance, and so on, I'm really happy.


Do you know when monticello was invented?
and working for real?

Stef


Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

stephane ducasse
In reply to this post by Frank Shearar-3
>>>
>>>
>> i am very happy that github exists.. and its features available only if you
>> put code there..
>> but if code is not on github, then what you will do?
>
> To rephrase the question: what do we still need to do before we can
> integrate (something like) github into a standard Smalltalk workflow?

how many man month engineer time do you have free for that?
Personally I prefer that
        FFI
        VM
        vector graphics
        …
        get pushed.

So git integration will come but on people free time.

Stef
Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Esteban A. Maringolo
In reply to this post by stephane ducasse
2013/3/11 stephane ducasse <[hidden email]>:

>> Every time I see smalltalkers forking away from established mainstream
>> solutions/practices I hear a big noise inside my head. On the
>> opposite, whenever I see stuff like the CI integrations, FileTree,
>> standards compliance, and so on, I'm really happy.

> Do you know when monticello was invented?
> and working for real?

Around 2004-2005 as far as I remember. And working for real at that
very moment, I guess.

Why?

Regards!


Esteban.

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Frank Shearar-3
On 11 March 2013 16:28, Esteban A. Maringolo <[hidden email]> wrote:

> 2013/3/11 stephane ducasse <[hidden email]>:
>
>>> Every time I see smalltalkers forking away from established mainstream
>>> solutions/practices I hear a big noise inside my head. On the
>>> opposite, whenever I see stuff like the CI integrations, FileTree,
>>> standards compliance, and so on, I'm really happy.
>
>> Do you know when monticello was invented?
>> and working for real?
>
> Around 2004-2005 as far as I remember. And working for real at that
> very moment, I guess.
>
> Why?

Presumably because it predates git :) But I think 2003 is the actual birth year.

But the counter argument is trivial: how many people use Monticello?
(At most, hundreds. In comparison to at least 10s of thousands using
git.) Does Monticello actually use any of the special syntax-awareness
it's supposed to bring to the table? (No.)

But the counter-counter argument is "it's good enough". (I disagree,
but I accept the argument that it does work well enough to actually
get stuff done, even if code review in Smalltalk is currently way
harder than in Ruby or Clojure, the other languages in which I most
commonly program. And they're so easy to code review because being
git-friendly allows me to see diffs and discuss diffs and use all
manner of cool tools that hook into git.)

frank

> Regards!
>
>
> Esteban.
>

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

stephane ducasse
Sorry I do not have the time to discuss. Busy writing report.
Our goal is to make sure that people can use Pharo to build real application.
Of course we would love to be as big as ruby and have 1000 of contributors.
So far this is not the case so …
Still by focusing on key assets we are making progress.

Stef


>>
>>>> Every time I see smalltalkers forking away from established mainstream
>>>> solutions/practices I hear a big noise inside my head. On the
>>>> opposite, whenever I see stuff like the CI integrations, FileTree,
>>>> standards compliance, and so on, I'm really happy.
>>
>>> Do you know when monticello was invented?
>>> and working for real?
>>
>> Around 2004-2005 as far as I remember. And working for real at that
>> very moment, I guess.
>>
>> Why?
>
> Presumably because it predates git :) But I think 2003 is the actual birth year.
>
> But the counter argument is trivial: how many people use Monticello?
> (At most, hundreds. In comparison to at least 10s of thousands using
> git.) Does Monticello actually use any of the special syntax-awareness
> it's supposed to bring to the table? (No.)
>
> But the counter-counter argument is "it's good enough". (I disagree,
> but I accept the argument that it does work well enough to actually
> get stuff done, even if code review in Smalltalk is currently way
> harder than in Ruby or Clojure, the other languages in which I most
> commonly program. And they're so easy to code review because being
> git-friendly allows me to see diffs and discuss diffs and use all
> manner of cool tools that hook into git.)
>
> frank
>
>> Regards!
>>
>>
>> Esteban.
>>
>


Reply | Threaded
Open this post in threaded view
|

Fwd: Code review tools?

stephane ducasse
In reply to this post by Esteban A. Maringolo
>
>> Do you know when monticello was invented?
>> and working for real?
>
> Around 2004-2005 as far as I remember. And working for real at that
> very moment, I guess.
>
> Why?

because pay attention when you talk about smalltalker reinventing the wheel.
So far with this argument we could say that we should all use eclipse because Java invented the ide, isn't.

So if you want Pharo to make progress, contribute.
Simple easy and with measurable impact.

Stef

>
> Regards!
>
>
> Esteban.
>


Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Code review tools?

Sven Van Caekenberghe-2

On 11 Mar 2013, at 17:39, stephane ducasse <[hidden email]> wrote:

> So if you want Pharo to make progress, contribute.
> Simple easy and with measurable impact.
>
> Stef

+100

Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Code review tools?

Frank Shearar-3
In reply to this post by stephane ducasse
On 11 March 2013 16:39, stephane ducasse <[hidden email]> wrote:

>>
>>> Do you know when monticello was invented?
>>> and working for real?
>>
>> Around 2004-2005 as far as I remember. And working for real at that
>> very moment, I guess.
>>
>> Why?
>
> because pay attention when you talk about smalltalker reinventing the wheel.
> So far with this argument we could say that we should all use eclipse because Java invented the ide, isn't.

In 2003, Monticello was the only game in town. It's now 10 years
later. You need to reevaluate your decisions sometimes. Concentrating
on application level stuff at the expense of infrastructure comes at a
cost. In particular, spending time on better code review tools will in
the long run accelerate your development velocity. This is supposed to
be a key Pharo trait. Now note: I said "spending time". That MIGHT
mean reinventing github's features. That MIGHT mean giving Camillo and
friends free reign to integrate git into a Pharoer's workflow.

But this kind of passive aggressive attitude is seriously
counterproductive. Because you're _not_ just telling people to shut up
and contribute. You're just telling people to shut up.

At any rate, Esteban, people _are_ working on Smalltalk/github
integration. Some of them are actually Pharo devs!

frank

Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Code review tools?

Esteban A. Maringolo
In reply to this post by stephane ducasse
2013/3/11 stephane ducasse <[hidden email]>:

> because pay attention when you talk about smalltalker reinventing the wheel.
> So far with this argument we could say that we should all use eclipse because Java invented the ide, isn't.

Well... Eclipse was a mutation from VisualAge... it took the best
parts of it. So in that case they did exactly that.
I don't want an Eclipse IDE Plugin for Pharo. I expect the Pharo team
to do that? Of course not! But I wont deny I think it could be a good
selling point.

> So if you want Pharo to make progress, contribute.
> Simple easy and with measurable impact.

I totally agree with that.

On the other hand there's no need to be harsh, we're all on the same
side, but please consider that contributing with a project is not only
a matter of producing code. Getting people aboard by advocating its
use also helps. IMHO, The user base is as important as the features.

Regards,

Esteban.

Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Camillo Bruni-3
In reply to this post by Frank Shearar-3
> Presumably because it predates git :) But I think 2003 is the actual birth year.

yet it shares almost the same model as git does. Sadly the tools don't
make proper use of it :). But that means that Monticello as a model isn't
that bad.

> But the counter argument is trivial: how many people use Monticello?
> (At most, hundreds. In comparison to at least 10s of thousands using
> git.) Does Monticello actually use any of the special syntax-awareness
> it's supposed to bring to the table? (No.)

indeed, that is one of the arguments that make git support for smalltalk
absolutely feasible. And even if there is going to be special object
support, I think we can store that in separate binary blobs if necessary.

> But the counter-counter argument is "it's good enough". (I disagree,
> but I accept the argument that it does work well enough to actually
> get stuff done, even if code review in Smalltalk is currently way
> harder than in Ruby or Clojure, the other languages in which I most
> commonly program. And they're so easy to code review because being
> git-friendly allows me to see diffs and discuss diffs and use all
> manner of cool tools that hook into git.)

I would love to run different branches on that topic:
- in image tools (such as torch)
- versioning model (such as git)

those two are basically orthogonal, but if you have a look at torch
you see that there is already quite a complex and rich tool available
for Pharo. It needs polishing I guess and depends on too many external
packages, but it is there and can server as a very good basis to go further.


For the versioning model we're running multiple branches as well:
- git (slow progress)
- libgit bindings
- filetree as a dialect agnostic fileout format (mostly completed)



Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

Camillo Bruni-3
In reply to this post by stephane ducasse
>>> i am very happy that github exists.. and its features available only if you
>>> put code there..
>>> but if code is not on github, then what you will do?
>>
>> To rephrase the question: what do we still need to do before we can
>> integrate (something like) github into a standard Smalltalk workflow?
>
> how many man month engineer time do you have free for that?
> Personally I prefer that
> FFI
> VM
> vector graphics
> …
> get pushed.
>
> So git integration will come but on people free time.

So I see, your git allergies haven't stopped yet
Reply | Threaded
Open this post in threaded view
|

Re: Code review tools?

stephane ducasse
In reply to this post by Frank Shearar-3

On Mar 11, 2013, at 6:04 PM, Frank Shearar <[hidden email]> wrote:

> On 11 March 2013 16:39, stephane ducasse <[hidden email]> wrote:
>>>
>>>> Do you know when monticello was invented?
>>>> and working for real?
>>>
>>> Around 2004-2005 as far as I remember. And working for real at that
>>> very moment, I guess.
>>>
>>> Why?
>>
>> because pay attention when you talk about smalltalker reinventing the wheel.
>> So far with this argument we could say that we should all use eclipse because Java invented the ide, isn't.
>
> In 2003, Monticello was the only game in town. It's now 10 years
> later. You need to reevaluate your decisions sometimes. Concentrating
> on application level stuff at the expense of infrastructure comes at a
> cost.

Apparently you did not read the documentation I wrote about the Pharo vision :)
too bad because I wrote it so that people understand that we are focusing on infrastructure.
This is already 3 years that we are focusing infrastructure.
I suggest that you have a serious look at what we are doing.

> In particular, spending time on better code review tools will in
> the long run accelerate your development velocity.

Yes may be

> This is supposed to be a key Pharo trait. Now note: I said "spending time". That MIGHT
> mean reinventing github's features. That MIGHT mean giving Camillo and
> friends free reign to integrate git into a Pharoer's workflow.
>
> But this kind of passive aggressive attitude is seriously
> counterproductive. Because you're _not_ just telling people to shut up
> and contribute. You're just telling people to shut up.

Ahhhh you were talking to esteban :)


> At any rate, Esteban, people _are_ working on Smalltalk/github
> integration. Some of them are actually Pharo devs!
>
> frank
>


12