wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 612 forks source link

[discussion] Monorepo all the things #4185

Closed pjreiniger closed 1 year ago

pjreiniger commented 2 years ago

TL;DR, merge Shuffleboard, *SmartDashboard, RobotBuilder, *Pathweaver and Grip into allwpilib

I am certainly on #teamMonorepo, but I know the community is pretty split on the topic. It is the trend that many tech companies have migrated to, including mine. Releases will become simpler, as you won't need to release allwpilib, then cascade the changes down to all the tools projects. I'm sure everyone can agree that the utility of a monorepo has paid dividends from the old way that cscore, ntcore, and wpiutil were managed. IMHO the dividends will continue if the tools were moved into this repository as well. I mean, it is called ALLwpilib for a reason 😃

I fundamentally do not believe in the statement "Well, we don't have any Shuffleboard maintainers, so it doesn't make sense". As long as these projects are intended to be released for the upcoming season, the are being supported whether you like it or not. Mass refactoring (like the proposed wpinet library and especially NT4) have an affect on these downstream tools, and validating that they work at the moment they are PR'd will save massive amounts of time during beta testing. Furthermore, bugs introduced in allwpilib updates can more easily be bisected (as happened with at least Shuffleboard this season). I've seen PR's in SB and SD that involve changes like "SpeedController" -> "MotorController" renames, which should be the responsibility of the initial refactor.

I put asterixis next to SmartDashboard and Pathweaver because I don't know when their sunset date is. I have high hopes for Waymaker replacing Pathweaver, at which point it would becoming obsolete, like OutlineViewer before it. SmartDashboard is ancient, and I'm curious how much it is being used. I threw Grip in because I think that there is still utility in it; PhotonVision won't always be magic bullet for what you want to do. We made our own vision pipeline for ball detection to get it up to 75 fps running with limelights new python utility, and did the opencv calls by hand. I would have much rather had my students make the pipeline graphically than me setting up / handholding them through the setting up the bones for the program. I don't think we live in a world where people will be generating java for use on the roborio, but it currently generates code that does not compille.

I think you could massage sysid into the conversation as well. Watching the development over the past offseason they disabled navx/rev/ctre portions until the library was released, while continuously updating the core library to dev releases / beta releases. I imagine you could #ifdef them out and toggle a #define when the ABI breaks and you are waiting on vendor updates. But for an initial pass, I think it makes sense to rule this tool out.

Pros;

Cons:

calcmogul commented 2 years ago

I fundamentally do not believe in the statement "Well, we don't have any Shuffleboard maintainers, so it doesn't make sense". As long as these projects are intended to be released for the upcoming season, the are being supported whether you like it or not. Mass refactoring (like the proposed wpinet library and especially NT4) have an affect on these downstream tools, and validating that they work at the moment they are PR'd will save massive amounts of time during beta testing. Furthermore, bugs introduced in allwpilib updates can more easily be bisected (as happened with at least Shuffleboard this season). I've seen PR's in SB and SD that involve changes like "SpeedController" -> "MotorController" renames, which should be the responsibility of the initial refactor.

The fact that forces people unfamiliar with the dashboards to fix breakage might be impetus to just drop the unmaintained tools alltogether. The only reason we haven't dropped them yet is they still compile and function.

I put asterixis next to SmartDashboard and Pathweaver because I don't know when their sunset date is. I have high hopes for Waymaker replacing Pathweaver, at which point it would becoming obsolete, like OutlineViewer before it. SmartDashboard is ancient, and I'm curious how much it is being used.

There isn't a date, but we want to replace them eventually.

I threw Grip in because I think that there is still utility in it; PhotonVision won't always be magic bullet for what you want to do. We made our own vision pipeline for ball detection to get it up to 75 fps running with limelights new python utility, and did the opencv calls by hand. I would have much rather had my students make the pipeline graphically than me setting up / handholding them through the setting up the bones for the program. I don't think we live in a world where people will be generating java for use on the roborio, but it currently generates code that does not compille.

GRIP is abandonware. Someone tried upgrading it to OpenCV 4.5 and failed. If people want it to stay around, they'd have to fix it up and maintain it.

I think you could massage sysid into the conversation as well. Watching the development over the past offseason they disabled navx/rev/ctre portions until the library was released, while continuously updating the core library to dev releases / beta releases. I imagine you could #ifdef them out and toggle a #define when the ABI breaks and you are waiting on vendor updates. But for an initial pass, I think it makes sense to rule this tool out.

We explicitly put SysId in a separate repo to segregate the vendordeps from the main WPILib codebase and avoid a circular dependency.

  • Apply a consistent styleguide between all the projects, which should make contribution easier (for instance, pathweaver uses standard java styleguide as opposed to hungarian member names). Each repository has different versions of checkstyle / pmd, and different rules enabled

That seems nice, but a lot of work.

  • All issues and PR's will have eyes on them. I don't know how often you guys check on things for the other repositories, but you all seem quick to respond to things in this repository.

We get Slack notifications for all issues and PRs as soon as they're opened. If we didn't respond, it's because we either don't have an opinion (various allwpilib stuff) or we don't know enough about the project to comment (the dashboards and GUIs).

  • All issues will show up in one place. I'll give you the argument of "no shuffleboard maintainers" for this point, but a quick response of wontfix or help wanted is better than things dying on the vine in the other repositories.

Since GitHub issue transferring exists and we see all issues ASAP anyway, whether they get opened in their respective repo or a monorepo doesn't matter to us.

Daltz333 commented 2 years ago

None of these reasons apply to documentation, so I'm safe!