search

wpilibsuite / allwpilib

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

[ci] Add workflow to build java tools for PRs #6698

Closed sciencewhiz closed 3 weeks ago

sciencewhiz commented 1 month ago

This can catch breaking changes earlier Builds Linux only for speed Builds and publishes artifacts in this file as it was actually faster then reusing the artifacts from the gradle file as those need the combiner run on them. It uploads the artifacts to Actions with a 1 day retention, because that allows the tools to be built in parallel, which overall sped things up. This also only allows relevant tasks in wpilib build to be run The tool builds are uploaded with a shorter retention time in case someone wants to do more extensive tests.

sciencewhiz commented 1 month ago

The PathWeaver failure is an example of this working, since PathWeaver doesn't currently build against allwpilib main (needs https://github.com/wpilibsuite/PathWeaver/pull/301)

Starlight220 commented 1 month ago

What is the intended flow here?

The tool PR can't be merged (or automatically tested) before the WPILib one, so this check has no way of succeeding on a PR that breaks the tools. The two PRs are coupled.

Is the intended flow "contributor opens PR that fails this check, contributor opens a PR to update the tool and (hopefully) tests it locally, override the failure and merge both PRs"?

I'm not necessarily opposed, but the flow should be agreed upon, especially by @PeterJohnson

pjreiniger commented 1 month ago

4185

My opinion is pull it all in here, and if it is "abandonware", abandon it.

sciencewhiz commented 1 month ago

I'm not necessarily opposed, but the flow should be agreed upon

My first intention was to help me make sure that I'm tracking what command PRs need RobotBuilder updates. Once I got that working, it was easy to extend to other tools.

My intention is to bring that discussion into the PR. Sometimes there may be a change that is unintentionally breaking, and can be fixed to make the PR not breaking. If the change is breaking, and it's something that we do want, I'd personally be happy with just an issue created so it's not forgotten. Each of the past few years, we've had to make last minute fixes to the tools based on breaking changes that were missed. Although there is the disadvantage that if the tool isn't fixed promptly, there may be additional breaking changes that are missed. It also can't be a required check for that reason, which makes expanding the monorepo attractive for that reason.

One future goal is to bring in the python build, to make sure everything is compatible with pybind. That's been an area that's often delayed python releases because there's been some new language feature that is not easily supported in python and it's not caught early.

sciencewhiz commented 3 weeks ago

My opinion is pull it all in here, and if it is "abandonware", abandon it.

Thad said the tools plugin would need significant updates to pull the tools in.