As WPILib has expanded over the past few years, so too has its pool of contributing developers. One thing that has become clear to me is that contributing to WPILib is an extremely valuable experience for students, as it teaches you how to interact with a living codebase at enterprise scale, complete with substantial CI, enforced style standards, and rigorous code review.
However, WPILib’s contributing guidelines are quite old and date from the days when the library was freshly-hosted as an open-source project. I’ve heard from multiple students that, while they’ve found contributing to be a rewarding activity, they did not feel that it was obvious that student contribution was desired, and they did not find that the guidelines adequately prepared them for the review process.
Clearly we need to improve here. The first step to improvement is planning - in keeping with the recent trend of “seeking CD feedback on new WPILib developments,” I figured I’d start a thread here to source community feedback on contributing guidelines.
So, what would you like to see in the way of contributing guidelines for WPILib? Be as vague or as specific as you like. Some ideas to kick off discussion, that have occurred to me:
A “student-to-contributor” tutorial sequence of some sort on FRC-docs, that details the “happy path” and likely pain points when transitioning from developing team code to developing open-source library code
A “WPILib architecture and philosophy” article that describes the core structure of the WPILib project and its motivation
Git hooks for auto-formatting available for new developers, so that PRs are less likely to get caught up on linting errors.
Good lord yes… I get it, some people are really hung up on linting and sometimes for good reason but it gets in the way of productive work at times for the sake of having some formatting opinions.
Contributions being welcome is definitely a message that should be getting out more.
I think it’s also important to mention that contributing to WPILib doesn’t require some really high-level programming knowledge; a programming student with a robot or two of experience should be able to handle most stuff–especially with the immense amount of support from the WPILib team. Proof: a significant amount of the current WPILib team are recent alumni, who got involved while being students and then “graduated” into the team.
This might’ve been changed already, but the requirement of implementing both C++ and Java changes drives potential contributors away and isn’t really true anyway: if needed, you’ll get help with the other language.
Issue triage/labeling should be improved: many issues don’t have a clear indication of what their status is, what exactly is intended, etc. Keeping issues up-to-date with sufficient information about what exactly is intended, any barriers for the issue to happen, and the like would make contributing easier. Marking issues as good first issue, component labels, and perhaps some sort of complexity level could really help new-ish contributors find issues to gain footing.
Feature requests should be addressed better and discussed earlier: I’ve seen multiple cases where someone starts working on an issue, and only when it reaches the PR review stage then it turns out that the change described in the issue isn’t desired. The time to discuss/debate feature designs should be at the issue stage, not after someone puts time into implementing it.
Community transparency/involvement can definitely be improved; these CD threads are definitely a step in the right direction in that regard.
TBA leverages git hooks for doing several checks before allowing upstream pushes. However, this is an optional step in the repo setup, since it requires some installations on the contributors machine. Repo Setup · the-blue-alliance/the-blue-alliance Wiki · GitHub
Re: git hooks, I’m partial to the NPM lint-staged package but the rest of the WPILib team is not keen on adding a package.json for autoformatting alone.
An ideal solution would be something with identical functionality to lint-staged, and an equally-simple installation step (i.e. a single shell command in the project root directory on the dev machine).
The most annoying part for users is configuring the linter, not running it. wpiformat requires installing python/pip and clang-format. On Windows, installing clang-format isn’t as simple as running the LLVM installer because Gradle autodetects its broken compiler and uses that instead of using MSVC. You have to run the installer, copy clang-format out of it and put it in PATH, then uninstall LLVM.
For Java formatting, we already have ./gradlew javaFormat and ./gradlew spotlessApply, and they’re documented in the main readme. They also get run by ./gradlew build and even subprojects like ./gradlew wpilibj:build, but no one runs those because they take too long and require a C++ compiler (Java calls into C++ code).
The golden model we use at work is a system where, as the first step of a code review, a single command is run on the files and all are instantly formatted to spec. And it’s never thought about or discussed after that.
But I’m all for keeping lint passes as required. I’m all too creative myself when it comes to code formatting, and I don’t want that unleashed on the world.
Yes. Also, start with a “Why should you contribute to WPILib?” hook.
Alternate things to discuss too - how to make/explore quick changes to wpilib. IE, in java at least, a quick answer is to copy-paste the contents of a .class file into your own .java file, find/replace names, and include that new one with modifications. Show the easy path to try out something new, then wrap back around to how to contribute that change back to WPILib.
Having public forum meetings where anyone can join in to hear about current activities, associate some faces or voices with the current contributors, and hear first-hand where the pain points are at, all could be helpful.
This is a bigger burden but something to consider… if someone’ serious about wanting to start but doesn’t know where or how, pair them up with an experienced person for mentorship. This requires quite a bit of extra time and formal dedication probably… but I can’t think of a better way to ramp someone from zero to productive.
I really like this idea. The difficulty we want to minimize is getting a valid development environment set up (where that includes installing hooks that handle linting/formatting so that you don’t have to think about it, in addition to whatever else we decide is appropriate). The closer we can get that experience to “run a single install task and then you’re ready to develop,” the better.
Is there any guidance provided on approvals and how those are done and specifically who the approvers are for different contributions? I feel like that’s a bit nebulous but that could also just be me not knowing that something exists.
This is super nebulous and is part of our internal process that probably needs some work (it being a volunteer project, we’re still very much on a model of “when people get around to it”).
We already use GitHub’s “code owners” feature to tag relevant reviewers to PRs. The real solution is maintainers finding the time to respond to reviews.
I don’t think we can get all the way there because CI installs clang-format through chocolatey on the commandline, and Visual Studio is preinstalled on the Actions instance. Users don’t have chocolatey, and idk if Visual Studio supports headless, noninteractive install.
Also doing a quick drive by to say - TBA went this way a few years ago and it’s been great. Being able to tell new contributors to run vagrant up (we use Docker + Vagrant - so substitute this for some docker-compose exec command in WPILib’s case) as opposed to downloading/installing a bunch of different pieces of tooling is helpful.
I think WPILib will probably have an easier time implementing something like Docker/Docker Compose and having a few commands built that run inside the container for building/testing/whatnot (I worked on a project that had this setup recently and it was a BREEZE to get up and running).
