CI: Use binstall, Always run tests/linux

[luigi311]

• Uses binstall to avoid building tauri-cli
• Sets tests and linux release to always run on push/pr
  • Useful to always catch regressions and testing new branches via docker
• Adjust the linux release workflow so it does not fail when a dockerhub username is not provided
  • Useful for other contributors to not get failed actions if they dont use dockerhub

[ikatson]

Uses binstall to avoid building tauri-cli
Wow, this looks like it might speed things up quite a bit. Love this, didn't know about binstall.

Sets tests and linux release to always run on push/pr

If I read this correctly, it will create junk in my dockerhub (ikatson/rqbit) on every PR etc. Don't like this if so. At least need to guard it somehow.

Adjust the linux release workflow so it does not fail when a dockerhub username is not provided. Useful for other contributors to not get failed actions if they dont use dockerhub
Seems alright if it doesn't impact the main repo

[ikatson]

I don't like this building / wasting CPU / resources on every single push to random branches

[luigi311]

The problem with limiting it to branches is now it wont run when others are developing. When you work off a fork you aren't supposed to do any work in main branch so you are supposed to create a feature branch that all your work is in that you then send the PR for. If we limit it to main branch then the new developer wouldn't know that something they changed is affecting the build process until they submit the PR. Ideally they would catch all the locally but then again that would defeat the purpose of the CI to begin with.

[ikatson]

Sorry I thought I commented on the test.yaml change. I'll expand in another comment

[ikatson]

Why split this up?

[luigi311]

Its only split up since i cant think of an easy way to keep them combined that accomplishes the end goal. I'll play around with this and see if we can keep this as a single piece.

[luigi311]

Converting to draft until i can figure out a good way to merge them.

[luigi311]

Wow, this looks like it might speed things up quite a bit. Love this, didn't know about binstall.

Binstall is great, i only heard about it recently during my dive into tauri 2. It will also fall back to doing a normal install if the binaries arent already hosted so it should be a transparent change for the most part.

If I read this correctly, it will create junk in my dockerhub (ikatson/rqbit) on every PR etc. Don't like this if so. At least need to guard it somehow.

Yeah this would create a separate tag for PR, the thought behind it is that it would make it easier to test for PR for you and the end users. We can remove the docker pushes if you want that way atleast the test and docker build is ran on PR to make sure the PR isnt breaking anything.

[ikatson]

There's a few changes here and I feel different about all of them. Let's break up into details. Feel free to merge the LGTM ones separately in a different PR, but there's one change here that I can't agree with yet.

Binstall change

LGTM, no concerns

Run tests on every push to every branch

Mixed feelings about this one, but I get the intention and I'm ready to try this out, so LGTM.

Dockerhub - replace "ikatson" with DOCKERHUB_USERNAME

LGTM, no concerns.

Run linux release on every push to every branch / tag and on every pull request.

Can't approve this. The intention was to build final artifacts (including docker) is just for release, not for pull requests and pushes to random branches.

Yeah this would create a separate tag for PR, the thought behind it is that it would make it easier to test for PR for you

I don't need docker to test it. I can build myself locally if needed. And I don't want a tag for unmerged PRs in the official dockerhub. I also don't need them for random branches that I create and push to all the time.

and the end users.

I guess you mean developers, not end users right?
Developers can build themselves on whatever system they are developing. Local "cargo build" should be enough, shouldn't it? If you need to test on docker for some reason, building docker locally should do the job

docker build is ran on PR to make sure the PR isnt breaking anything.

As mentioned above, the intention about docker builds wasn't to make sure it's not breaking anything but to create artifacts for final release. For other purposes test.yaml should be enough.

If you really need to test docker builds, e.g. you are changing the actual docker build process, that's a different story, but it was so rare so far that wasn't an issue.

[luigi311]

That all sounds good to me, ill go ahead and revert the change for the change for the non main linux release and see if i can combine the two docker build sections and let you know. Thanks for taking a look at it.