CI cleaning

[AngelOnFira]

This PR is to clean up CI a bit, and separate out some tests into their own files. It also adds a RustFmt check.

Specifically, all of the tests for postgres/mysql/sqlite were all moved to respective test files (postgres.yml for example). All of the general unit tests, clippy, and other generic items were added to a rust-ci.yml file.

[AngelOnFira]

This is the only section I added to the CI files, the rest was just rearranged.

[billy1624]

I wonder what this will do? Perform cargo fmt and push the formatted code to PR / master?

[tyt2y3]

I think it will fail if rust fmt formatted anything.

[tyt2y3]

Thank you for the proposed changes.

Can you explain what is the advantages of breaking down the workflows?
Because I often rely on that passing badge to tell me whether things are fine.

I also see that rust fmt is pretty common across rust projects, but one thing I dislike about it is it often create unnecessary line changes that would make merging branch and resolving conflicts harder. I am open on this, and may be we can try for a while and see whether it causes trouble.

[billy1624]

It's quite counter intuitive that GitHub collapsed all workflows instead of expanding all. Making it difficult to see it's all green or some failed :((

[AngelOnFira]

Thank you for the proposed changes.

Can you explain what is the advantages of breaking down the workflows? Because I often rely on that passing badge to tell me whether things are fine.

I also see that rust fmt is pretty common across rust projects, but one thing I dislike about it is it often create unnecessary line changes that would make merging branch and resolving conflicts harder. I am open on this though, and may be we can try for a while and see whether it causes trouble.

Good points. My original idea was to add fmt, since it wasn't running automatically. It is interesting that it was pretty much already formatted, with only one file that needed changes :P I have a pretty strong opinion that any public crates that wants to be used by lots of people should certainly use fmt, since it makes it far more universal for people to read later if they need. Also, I'm not sure I understand your point about "unnecessary line changes", it will only change lines that you change right? Won't that already conflict if people are also changing those lines? I work on Veloren, a game in Rust, and we really haven't run into this issue.

I did get quite carried away with the entire refactor though 😅 the way I saw it, the file was quite cluttered, and if I wanted to know how just the Postgres tests were running, the code for it was far apart. I'm definitely ok to revert the change into multiple files, I see how it could be detrimental in terms of the badge.

It's quite counter intuitive that GitHub collapsed all workflows instead of expanding all. Making it difficult to see it's all green or some failed :((

It does show when a certain section fails, otherwise you can assume it's all green. In my opinion, this is a lot easier to make sense of than all tests in the same category. For example, you'd easily be able to see that maybe just the sqlite tests are failing, while all the postgres/mysql tests are fine. I think this is how other projects tend to operate, but I'm not sure.

TL;DR, my main change that I want to try and keep is running fmt alongside clippy, but I'm ok to revert the file changes if my proposed benefits don't outweigh the downsides.

[billy1624]

Hahaaa we use cargo fmt a lot. But it's done manually loll

[tyt2y3]

We can give it a try. But let's put rust fmt before everything so it fails fast.

The current workflow also have two stages, so if compilation fails, subsequent tests would abort.

I tried to look for a combined status badge for all workflows, but couldn't. Otherwise I am fine with breaking it up.

[billy1624]

So we basically keep the unified old workflow and add a cargo fmt stage before it :P

[AngelOnFira]

Hahaaa we use cargo fmt a lot. But it's done manually loll

The new method won't create any commits in CI itself. You'll still have to manually format files, and commit the changes.

Let me first just make a prototype of what the readme could look like with separate badges. But unless you think that it looks ok, I'm good to just revert those changes.

[AngelOnFira]

I guess it wouldn't be the best, you could see the rendered version here.

[AngelOnFira]

I'll revert the main file changes for now, and just leave the fmt changes in. If the file separations end up being desired, I'll create a separate PR for it. I'll see if I can find examples of other projects that do it nicely.

[AngelOnFira]

Alright, everything should be good and simple 😃 I appreciate you both taking the time to discuss this!

[tyt2y3]

Sure thanks! I certainly prefer the simplicity of one badge. May be we'd encounter some third party services that does this.

[billy1624]

Thanks!! @AngelOnFira :))