Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Jun 8, 2024

Conversation

sciencewhiz
Copy link
Contributor

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 and others added 7 commits June 3, 2024 21:39
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
to be run
The tool builds are uploaded with a shorter retention time in case
someone wants to do more extensive tests.
)

Doesn't make sense in edu.wpi.fields, and resources are already in
edu.wpi.first.fields
This will require changes in Shuffleboard and Pathweaver
@sciencewhiz sciencewhiz requested a review from a team as a code owner June 4, 2024 14:39
@sciencewhiz
Copy link
Contributor Author

The PathWeaver failure is an example of this working, since PathWeaver doesn't currently build against allwpilib main (needs wpilibsuite/PathWeaver#301)

.github/workflows/tools.yml Outdated Show resolved Hide resolved
.github/workflows/tools.yml Outdated Show resolved Hide resolved
.github/workflows/tools.yml Outdated Show resolved Hide resolved
@Starlight220
Copy link
Member

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
Copy link
Contributor

#4185

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

@sciencewhiz
Copy link
Contributor Author

sciencewhiz commented Jun 4, 2024

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.

@PeterJohnson PeterJohnson merged commit a0efc9c into wpilibsuite:main Jun 8, 2024
31 checks passed
@sciencewhiz sciencewhiz deleted the BuildRB branch June 8, 2024 22:18
@sciencewhiz
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants