ODK 1 Developer calls bring together the developer community to discuss issues that concern us all. Everyone is welcome to participate in these calls.
The calls are held on the first Wednesday of every month on UberConference from 14-15 UTC. We try to send out a reminder on Slack the day before.
We put the agenda, audio, and transcript of every call at https://docs.google.com/document/d/1hszoTRzWG5W04JXgcBzE7BcdEZGB75lA8tftTNZ5lzA
Our next call will be Wed, Oct 3rd, from 14-15 UTC (see in your time zone). Will you be there?
- I will be there!
- I can't make it this time but I'm interested in future calls
If there are topics you would like to add to the agenda, please comment below.
I've had it on my list for a long time to write up a document on how we approach code review. As we've added more maintainers, I think it would be helpful to provide a bit more standardization between us and share best practices we've each developed. One of my main goals would be to make it less intimidating to participate in review so that more people try it.
Let's spend this time generating some content for that document.
I think it would be very helpful to discuss with both code reviewers (primarily @Grzesiek2010, @Shobhit_Agarwal, @ggalmazor, @yanokwa, @dcbriccetti these days) and folks who have been contributing a lot and may become maintainers (for example, @dexter21, @Mickys0918, @Saumia, @xsteelej, @joeldean, @jpknox, @Akshay_Patel, @zestyping and others!).
Questions to consider:
- Has everyone read the code review process section on the various tools? e.g. Collect, Briefcase
- What would be a good single location to put this so the projects can link to a common resource?
- When submitting a PR, what feedback is most helpful, least helpful? What is a helpful format to receive it in?
- How do we prioritize review?
- As a reviewer, how do you start when the PR is large?
- If someone only has a little bit of time for a review, what should they look for?
- When do we close a PR?
Hi! I have a conflict that prevents me from joining this call on time, but I hope I can contribute a little bit to this discussion.
Here are a few things that worked well in my previous experience developing a code review process:
Design documents can be really useful for substantial changes, long before they get to the code review stage. The existing PR template has a few of the elements of a design document—though, for a big change, it feels incomplete and cramped, whereas for a quick fix, it feels a bit tedious. I wonder if there's a way we could incorporate design documents for larger changes, while streamlining the PR process for smaller ones. A typical design document template might include:
- Alternatives considered
- Usage examples (for an API or developer-facing change)
- User-visible changes, UI rationale
- Testing plan
- Security impact
- Maintenance impact
Providing guidance to the reviewer can save time, and make starting the review less intimidating. The PR author could say:
- Where's the best place to start reviewing? (e.g. the core class or core interface that expresses the concepts you need to understand the rest of the PR)
- What are the most interesting implementation decisions that were made? (e.g. the obvious choice might have been A, but I did B for the following reasons...)
- What are the highest-risk aspects of this change? (both as it affects review—what code deserves careful scrutiny; and affects testing—what needs careful testing to ensure it didn't break)
A reviewer, conversely, can then focus on areas of risk, or on identifying areas of risk or potential confusion and looking to see that they are well covered by tests and documentation.
ahh unfortunatelly I can't attend today...