Post-Retrospective: Code review tool discussion

[Sebastian Brudziński]

Hello everyone,

as an outcome of our sprint retrospective meeting, I'd like to start the
discussion about current code review tool (Crucible) and the possibility
to replace it with another tool. Despite some very obvious lack of
features in Crucible (such as LOC added/changed in review) there are
also some flaws displaying the proper changes, especially when a few
changesets are added or when a file happened to be
deleted/moved/renamed. Another issue that was raised about Fisheye is
that it is really difficult to track the progress on a review (when was
the last changeset added? which comments were already addressed?). While
it is not a big issue for your own reviews (since you pretty much
remember what you have already done and which comments are new), for
someone that is taking over the work or for someone who just wants to
take a look and quickly estimate the effort required to address all
remaining comments, this is quite difficult.

What I'm asking for in this topic is to share your own experiences using
Fisheye. Do you see the same (other?) issues with Fisheye or do you
think it is actually working quite well? Do you know any other
code-review tools that could potentially be better?

One tool that I've recently seen and that may be a good candidate is
ReviewBoard (http://demo.reviewboard.org/r/).

Please note that this is not a request to change the code review tool
(yet)! This is a request to share your opinion about existing tool and
names/links of other tools that may work better than the current one.

Kind regards,
Sebastian Brudzinski.

[Brandon Bowersox-Johnson]

Hi everyone,

Sebastian, I’m very glad you raised this because it does seem to be an issue for many of us. I’m new to Fisheye and I have found it confusing myself.

As one initial step, I’d be glad to host a WebEx meeting where anyone interested can get together and share Fisheye questions and solutions. If anyone thinks they are a “pro” user, we should make sure they attend.

One specific problem I have with Fisheye is that the “Reviews” tab inside a JIRA issue does not work for me. It gives a yellow warning about authentication, and trying to authenticate generates a 403. Screenshot is attached. I’m wondering if this works right for anyone else?

In terms of picking a different tool, it seems like the industry has a few other smaller players but Crucible is a leading solution. It also has a few benefits going for it: we already have invested in getting it running and over time it may improve as it integrates with JIRA even better. I would love if the product gets better, because parts of it are definitely clunky.

-Brandon

[Łukasz Lewczyński]

Hi,

The FishEye is quite good review tool in a situation where a developer
creates new review, adds several files, adds few reviewers and there is
nothing to update/modified/changed. But this kind of situation is very
rare. The very often situation is when the developer need to add more
changes into the review because of added comments from reviewers.

Is quite hard to read review when some files (classes) have been renamed
or moved into other package. It is even worse when we have a review,
someone adds comment to rename class, a developer renamed the class then
someone else adds comment that this class should be in different
package, the developer move the class and in the end the review looks
very bad and it is hard to read.

Also if the review contains a lot of comments there is a problem to find
what problem (raised in comment) is already fixed and what problem has
to be resolved. Sometimes after adding new content into the review,
comments are hidden but not all.

I see that we use 4.0.4 version of FishEye and on atlassian side there
is version 4.2.1. Is it possible to update FishEye? Maybe some of our
problems have been resolved?

- Lukasz

[Brandon Bowersox-Johnson]

Hi Łukasz,

I’ll talk with Josh and the team about upgrading to the latest Fisheye. The release notes say that the user interface is improved:

“Updated Edit Review dialog: One of the screens we know you interact with the most is now redesigned to bring you a more user-friendly experience and be more consistent across Atlassian standards.”
- https://confluence.atlassian.com/fisheye/fisheye-4-1-release-notes-827346009.html

Upgrading is a simple change we can make that might improve some of the pain points of Fisheye. Thanks for suggesting this!

-Brandon

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/6627b721-fd18-2da5-b228-1d229a159e21%40soldevelo.com.
For more options, visit https://groups.google.com/d/optout.

[Weronika Ciecierska]

Hi everyone!
At our last retrospective meeting Crucible & changing review tool topic was brought up again.
Both developers and reviewers find it difficult to use Crucible, especially when there is a lot of changes and modifications done during the review process.
One of the problems mentioned during our meeting was the fact that Crucible is adding unrelated changes to the reviews (it was mentioned in https://answers.atlassian.com/questions/212874/diff-filtering-of-other-developers-commits-not-related-to-review).
Another inconvenience is that after renaming any file and pushing new changes to the existing review, all new changes are shown in a separate file - it can be really confusing for reviewers. There is no info that this file was renamed, it is shown as new added file.

Please let me know if you have experienced similar problems with our review tool. Do you now any good alternatives for Crucible? Our previous discussion here was pretty short, I would love to hear more people opinions about Crucible!

Regards,
Weronika

Weronika Ciecierska
Software Developer
wciec...@soldevelo.com

SolDevelo Sp. z o. o. [LLC]
Office: +48 58 782 45 40 / Fax: +48 58 782 45 41 Al. Zwycięstwa 96/98 81-451, Gdynia
http://www.soldevelo.com

Place of registration: Regional Court for the City of Gdansk KRS: 0000332728, TAX ID: PL5862240331, REGON: 220828585, Share capital: 60,000.00 PLN

[Nick Reid]

Thanks for bringing this up Weronika

I agree that Crucible makes it difficult to really review a large change

For what its worth, I think GitHub's code review tools are really seamless — the only issue being is they are focused on people working on branches....

Would it be reasonable to trail a different workflow for a sprint after the 3.0 drop?

Nick Reid | nick...@villagereach.org
Friendly Neighborhood Spiderman, Information Systems Group

VillageReach Starting at the Last Mile
2900 Eastlake Ave. E, Suite 230, Seattle, WA 98102, USA
CELL: +1.510.410.0020
SKYPE: nickdotreid
www.villagereach.org

---

From: openlm...@googlegroups.com <openlm...@googlegroups.com> on behalf of Weronika Ciecierska <wciec...@soldevelo.com>
Sent: Friday, February 3, 2017 7:36:18 AM
To: OpenLMIS Dev
Subject: [openlmis-dev] Re: Post-Retrospective: Code review tool discussion

 

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/375a36e1-e0ff-4c19-b7d0-54adc6225edc%40googlegroups.com.

[Brandon Bowersox-Johnson]

Nick’s suggestion about a trial change to our workflow after the 3.0 release is intriguing. Once we have an official release, it’s just as likely that any bug fixes or improvements might come from people who are involved with the in-country implementation(s). These might be people we’ve never met, and who don’t already have credentials to commit to master directly.

 

It makes sense use GitHub’s code review tools for all of the minor point releases (3.0.1, 3.0.2, etc) once 3.0 is released. That could be an open, transparent way for anyone to participate in helping patch and fix the software that has shipped. And it could help us test out whether we like the GitHub review tool and branching better. Of course we could also do a trial with the larger teams working on 3.1 as well.

 

There are quite a few stakeholders who would need to weigh in, but I’m open to it.

 

-Brandon

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/BN6PR02MB26258AE910E66388B12B776E94400%40BN6PR02MB2625.namprd02.prod.outlook.com.

[Josh Zamor]

Feature branch development is something we tried in the beginning and we largely had issues with it within our teams:

• more than a couple days between when work was started, and when it showed up on GitHub
• large merge commits back to master
• without it available on Master, it was invisible to CI and all the benefits of this:
  • services not published to DockerHub, Libraries and Ext. Modules not published through Maven
  • no automated testing, including contract testing (fixable yes, though requiring further investment)
  • instant feedback at test.openlmis.org (further investment here is already needed and being planned)
  • no Sonar analysis, no ERD diagrams, no docs published

For these reasons we’ve been discouraging it.  Some I believe still use it successfully and sometimes a branch just makes sense - like sharing a bit of code to get help when it would break the build.

For developers that are not part of one of the active teams, I agree that feature branches and pull requests make sense.  For the active teams, what are we going to do differently to avoid the issues we encountered before?

For the record I’m not that excited about Crucible either.  This is a recurrent discussion about which code review tool is the best and I’m pretty certain that no matter what tool we change too, we’ll still find something to be lacking.  Perhaps we should be submitting improvements to Crucible (only semi-joking)?

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/D2053854-5E5E-43F9-9BDC-D3A7DAC7889F%40villagereach.org.

[Nick Reid]

Yea, I'll admit that I'd complain no matter what happens... and staying within the JIRA stack from start to finish is kinda rad (i mean buggy, but rad)

Nick Reid | nick...@villagereach.org
Friendly Neighborhood Spiderman, Information Systems Group

VillageReach Starting at the Last Mile
2900 Eastlake Ave. E, Suite 230, Seattle, WA 98102, USA
CELL: +1.510.410.0020
SKYPE: nickdotreid
www.villagereach.org

---

From: Josh Zamor
Sent: Monday, February 6, 2017 10:08:59 AM
To: Brandon Bowersox-Johnson
Cc: Nick Reid; OpenLMIS Dev