8 ms·

Ask HN: How to handle code reviews with a visually impaired coworker?

74 points by keufran 7y ago

Hello World, My dev team has switched to a workflow using merge requests and code reviews (mostly by commenting the merge request). Our tool is Gitlab. I needed to embed a visually impaired co-worker in my team but I face some difficulties: - Gitlab accessibility seems to be really bad and my co-worker is unable to use the interface to create Merge Request (he uses accessibility tools, of course). I dont'even speak about reading and writing comments in merge request. - I don't know how to handle the code review process with him. We could do physical Code Review sessions, but it's difficult because I've a very chaotic schedule and so it's difficult to find a common timeslot. Furthermore, it's very difficult for my co-worker to handle all the remarks in one session for any Merge Request with a significant amount of code. - I need to keep in place the existing tooling for the rest of the team Does anybody knows of tools interfacing with Gitlab (or the git repository) or methodologies that could help us ?

ainiriand 7y ago
The best tool would be another human, let him/her do the code review in parallel with another dev. Is that a possibility?
catchmeifyoucan 7y ago
Have you looked at mobile? Perhaps there's a mobile app that might actually be more readable.
geocar 7y ago
Try gitlab-cli[1] or cli-gitlab[2]. [1]: https://github.com/vishwanatharondekar/gitlab-cli https://github.com/vishwanatharondekar/gitlab-cli [2]: https://github.com/mdsb100/cli-gitlab https://github.com/mdsb100/cli-gitlab
- keufran 7y ago
  I wasn't aware of these tools. I will have a look on them, thank you very much.
dyeje 7y ago
Have you tried reaching out to GitLab team about improving accessibility?
- lbotos 7y ago
  I work at GitLab. I just passed this along to the UX team. Thanks for raising the question!
  - keufran 7y ago
    You're welcome. It's great if just raising the question help other people :)
- trampi 7y ago
  With a bit of luck, GitLab C-Levels will read this and priorize it. Wouldn't be the first time here.
- keufran 7y ago
  No I didn't. I even don't check for myself if accessibility is really bad, but I trust my coworker on this.
awinder 7y ago
Are you code-reviewing all code for your team? Maybe time to loosen up the reigns and let the team own the outcomes, I'm sure they have way more available time to work with this developer 1-on-1 (and they can treat code reviews as an interruptible task, which it should be -- it's the last step before return on investment)
- keufran 7y ago
  No, I don't review all team's code. A point is that he will work alone on a very specific piece of code. He is junior on the technology, so I'll review mostly for educational purpose.
ydnaclementine 7y ago
Could the person check out the branch, and their editor/IDE show the diffs in a way for them to understand? If there is a line that could be improved, maybe they get the line number from the IDE, and have a single comment in the PR instead of trying to do inline It's tough, I feel for them
gpm 7y ago
What about just downloading and adding comments to the text based diff representation (and maybe email or something to send things back and forth).
debt 7y ago
"We could do physical Code Review sessions, but it's difficult because I've a very chaotic schedule" Aren't you a developer? You're not a manager? Can you explain why you have a chaotic schedule? If you don't code, why are you doing code reviews then? Seems like that's the fundamental problem right there. You said so yourself, physical code reviews would solve it. So that's the solution, the problem is your schedule, fix your schedule. You're making this person accommodate your schedule(in addition to their own), because you don't want to do your job. Again, unless I'm missing something.
- jrockway 7y ago
  Programming is to software engineering as telescopes are to astronomy. It's one tool that you use, but is not the only tool you use. Planning what you're going to make is as important as making something; as you move higher up in your career development, you will realize that a few hours of talking can save months of work. It seems "productive" to spend months at a time programming, but if nobody uses what you produced, how productive is it really?
  - deleted 7y ago
    [deleted]
- keufran 7y ago
  You're absolutly right, and I'm looking to improve this. However this is not the only pain point. The amount of remarks you can do in a code review can be huge, and I don't expect my coworker to be able to remind all what I say. So I also need a "place" where to put the remarks that he could refer to at his own pace.
  - traviscj 7y ago
    Maybe a digital voice recorder thing (like [1]) so neither of you has to take explicit notes but can still capture all the discussion? [1]: https://www.amazon.com/Sony-ICDPX370-Digital-Recorder-Built/dp/B06XFTWCBJ https://www.amazon.com/Sony-ICDPX370-Digital-Recorder-Built/...
DoofusOfDeath 7y ago
Would it be possible to involve him in the process of looking for solutions? He might have a head-start in terms of familiarity with existing tools. And he's best positioned to get an early sense of which solutions are (from his perspective) viable. To be clear, I'm not suggesting he should have to solve this by himself, especially in his off-hours. Nor should it necessarily crowd out the work he wants to be doing. But logistically it makes sense for him to help with the task.
- keufran 7y ago
  Yes, of course he will be involded by suggesting, trying and validating the solutions.
jareds 7y ago
I'm a totally blind developer and I find the easiest way to do code reviews is to use git format-patch on the branch containing the code. I read the patch files in a text editor. Perhaps comments in the pull request referencing a commit and line would allow the developer to get the required context from the patch files?
- fernandotakai 7y ago
  i hope this is not an intrusive question -- but how do you usually write code? with a screen reader?
  - jareds 7y ago
    Yes, I use Jaws for Windows. I also make heavy use of WSL for command-line stuff.
    - randomvectors 7y ago
      Kudos to you. Speaking as someone who has taken their eyesight for granted, it's absolutely incredible that anyone can do this at all.
      - skyyler 7y ago
        How patronizing.
        mpoteat 7y ago
        What did you find patronizing about the statement?
    - murph-almighty 7y ago
      Maybe I'm mistaking you for someone else but did you write a blog post about this?
    - stallmanite 7y ago
elil17 7y ago
There’s probably not an existing technical solution to this problem. You either need to get GitLab to fix it, create some kind of interface, or rework your schedule to accommodate in person reviews.
- keufran 7y ago
  I hope Gitlab will fix this (it really should) but I need a solution in the meantime...
bluGill 7y ago
If you are in the US stop everything and call your companies' lawyers. I suspect they will tell you nobody is allowed to use this workflow until you solve the problem. ADA in the US is strong and you don't want to be sued. If you pay for gitlab support ask your lawyer how much liability gitlab has should you be sued as well, and feed that back to your contacts.
- ccrush 7y ago
  This is an important factor many people neglect. It's so easy to end up on the wrong end of an AXA suit in this case. Many people forget how having a minor difference in treating a disabled colleague can mean the company is going to get sued for a whole shitload of money. Especially true when talking about developers who command a relatively high salary compared to most.
sn 7y ago
I work with a blind developer. We create personal feature branches, file a ticket for merge requests in our ticketing system, code review them directly in the ticket or via email, and then rebase the feature branch onto master. We don't, but you could plausibly do reviews directly in git via editing the file with any comments. I've been wanting to try gerrit however: https://gerrit-review.googlesource.com/Documentation/dev-design.html#_accessibility_considerations https://gerrit-review.googlesource.com/Documentation/dev-des... Please report back on whatever you end up doing. One caution I have is that long term you should consider moving away from the gitlab tools if they are unable to fix accessibility concerns, so that everyone is on equal ground. I also don't know how flexible he is on which browser or operating system he uses; he may want to try some others to see if they work any better.
- woodrowbarlow 7y ago
  gerrit is great because rather than invent their own code review process, they've basically implemented a gui for the same underlying process that you'd use in the command-line (git-format-patch, email, and inline comments). this means you don't have to use gerrit to participate in gerrit-based code reviews. because of this, i would imagine that it is very friendly to accessibility tools for programmers.
- keufran 7y ago
  Thanks for sharing. I'll do my best to do a follow-up.
lbotos 7y ago
@Keufran, I didn't see your email in your profile. If you email me at my work email in my profile, I'd be happy to put you in touch with our UX team directly.
- keufran 7y ago
  Thank You @lbotos; I will email you as soon as I'm back to work.
kraig911 7y ago
I've worked with some brilliant disabled or handicapped developers. I hope Gitlab and well our community to be honest will start truly making open accessible tools. I feel we push for shipping production stuff without truly understanding the cost of it. Because once something is out there it's about the next feature and not making things more accessible.
pgroves 7y ago
Everyone on the team would benefit from strongly preferring short (single change) pull requests. Complicated PRs have a high cognitive load for anyone and this issue is highlighting that problem, not causing it. This is a good excuse to enforce a policy to package changes as small, separate PRs whenever possible. Although I might be reading too much into your comment about "significant amount of code" being the tipping point. I generally think PR tools are all based on the same bad, diff-centric workflow that doesn't capture the narrative of the changes while bludgeoning the reviewer with minutia up front. The only defense my team has found is by making bite-sized PRs whenever humanly possible.
natpalmer1776 7y ago
Like any product design question (which I feel this qualifies as), it would be a good idea to ask him (the end user) "What workflows have worked well for you before?" Listen to his response and get a clear idea of what does and does not work for his needs, then build your proposed solutions around that.
- keufran 7y ago
  Yes, I clearly need to understand his actual workflow to know where (and how) to "hook" the review process. Thank you for the feedback.
Nextgrid 7y ago
There are GUI clients like Git Tower (no affiliation, other than being a former user) that support GitLab and might have better accessibility. They have a free trial so maybe give it a shot? Since it’s a native desktop app it might integrate better with a screen reader.
steve_adams_86 7y ago
I don't have many suggestions, but I think it's great that you're making an effort to make this work for your coworker. I can tell this would be a challenge. Getting people going with merge requests can be hard even with good vision. Is it possible for this person to do merge requests with another person present, like pair programming? It might seem counterproductive, but more eyes on the code can't hurt and it'll hopefully fill any accessibility gaps until this person finds better tooling to solve this problem.
codyb 7y ago
I don’t want to hijack the thread by any means, but after reading, I’m super curious. Can a blind developer make use of a modal editor like ViM? Would it be counterproductive? I’m pretty into the idea of accessibility but haven’t tried a screen reader myself. I definitely should but I bet the current app I’m working on would suck as it’s a sea of divs (I’m trying to change that by using headers, footers, and section elements). Really powerful stuff reading what some of the developers in this thread do. I’ve always understood that the screen readers zoom by, it’s all very interesting to someone who takes for granted what they have.
- mwcampbell 7y ago
  Yep, some blind programmers use vim. One of my blind programmer friends uses neovim and tmux, or at least he did a couple years ago when he blogged about a related utility he wrote [1]; I haven't asked him what he currently uses. As for the sea of divs in your application, here are the most important things you should do: 1. Don't use a div or span for a clickable element that acts like a link or button, unless you give it the appropriate ARIA role and handle keyboard focus. I'd say it's better to use a real link or button unless there's some reason that absolutely won't work with your CSS. 2. Put an ARIA main landmark at the start of the main page content. This can be a div with role="main", or you can use the main tag. I don't think it matters anymore which one you use. 3. If a page has section headings, use real heading tags (h1-h6). Hope this helps. Thanks for paying attention to accessibility! [1]: https://www.thewordnerd.info/blog/tma-the-tmux-automator/ https://www.thewordnerd.info/blog/tma-the-tmux-automator/
DoreenMichele 7y ago
There is a group called Blind Dev Works: https://groups.google.com/forum/?nomobile=true#!forum/blind-dev-works https://groups.google.com/forum/?nomobile=true#!forum/blind-... It's small and low traffic. You are welcome to join.
mwcampbell 7y ago
I appreciate that you're trying to help your coworker, but in this case, I don't think you and your team should have to modify your process to accommodate him. In particular, doing code review in-person, possibly using a voice recorder as was suggested elsewhere in this thread, is not necessary. After all, the asynchronous nature of online code reviews is a good thing; it means the whole team can get involved on their own schedule. Also, electronic text is the universal format, accessible to everyone regardless of disability. Imagine if you or one of your other teammates were deaf, or had a severe speech impediment. All of you, including your visually impaired coworker, would still be able to collaborate through text. I'm not surprised that he's having trouble with the GitLab web interface for merge requests. I'm going to assume that he's very proficient with his assistive technology and has tried every possible work-around on the website itself. GitLab also has an API, and someone linked to a couple of CLI tools elsewhere in this thread. If those don't help, your coworker should be able to hack together something that works for him. Or, if you really want to help him, talk to him about what kind of interface would work well for him, then hack it yourself. Just don't contort your team's process or your schedule around him. It may seem that I'm being too tough on your coworker. But I'm visually impaired myself (see my profile), and have several blind friends who are programmers. I don't think any of them would ask for in-person code reviews as an accommodation because of an inaccessible web interface. Again, I appreciate that you want to help your coworker; I'm just trying to steer you away from what I believe is a misguided solution.