Reference application JIRA workflows

Hi,

In the reference application’s jira project, a ticket goes from In Development to Ready For Testing, i think it should be In Development -> Needs Review -> Ready For Testing or back to In Development, what do others think?

Are you seeing any specific problem related to code review or QA testing that the current workflow doesn’t cover?

I could imagine having an optional step for “request code review” after In Development. (Not all UI-layer code in the reference application necessarily needs to be code reviewed.)

In general I want to nudge us away from a workflow where anyone thinks “the dev team is done, now let’s throw this over the wall to the QA team”.

Would there be an obvious way for people to know the difference?

@darius, i guess what am trying to say is that the default workflow should be In Development -> Needs Review -> Ready For Testing, but you can still be able to skip need review if not necessary depending on how the workflows in jira have been set up, you can have more than one status options to go to from one.

@wyclif, does “Needs Review” mean “Needs Code Review”? So really you’re suggesting “Ready For Code Review” and “In Code Review” (for consistency with everything else.

My first reaction is that I don’t like this idea: it takes what is already a long sequential pipeline of stages, and makes it even longer, and basically makes developing code for the reference application more tedious. (Also, I’d hope that code review and testing can be done independently, and not necessarily in that order, so I dislike introducing a bottleneck, since code review has typically been the thing we’re slowest at.)

About what should or should not be code reviewed, my quick answer is actually that anything in the general reference application codebase that doesn’t introduce any public API, business logic, or data model, doesn’t require code review at all. (It doesn’t hurt to have it, but given limited resources, it’s something I’d personally be happy to sacrifice.) Anything that does introduce a service layer should probably have a specific module ticket, right?

1 Like

What do others think?

Our problems with code review stem from:

  1. Not having clear guidelines/goals/process for code review.
  2. Not empowering more devs to help with code review.

How many devs have done code review for OpenMRS in the past six months? How does that number compare to three years ago? If we ask 20 OpenMRS devs to name the goals of code review & steps to review code, do you think we would get less than 20 different answers?

I don’t think we should be reducing the amount of code review; rather, I think we should be addressing the two problems above and, as a result, using fewer of our resources to perform triple the amount of code reviewing we’re currently doing. I would love to see a day where nearly every /dev/3+ is routinely performing code reviews, we have a suite of different code review types (quality, security, exceptions, clarity, conventions, etc.) and a Sonar-like tool helping focus our review efforts to maximize the effectiveness of code review (including occasionally reviewing code within the reference application UI).

I do agree with @darius, that we don’t need to be reviewing every line of code and the solution for code review is not via a increasingly prescriptive JIRA workflow. Overly prescriptive JIRA workflows tend to irritate more than help when they don’t match real life needs. Allowing a dev to request a code review via JIRA sounds great, but I wouldn’t force a code review step via JIRA. Even in openmrs-core, we only lasted a few weeks with a code review step before we ended up with a “post-commit review” status because code review was becoming a bottleneck.

1 Like

So should we mark an issue “Ready for Testing” once we completed working on it?