[CLI] Check with multiple arguments #91
Conversation
|
Conflict with #90. Depending on the one merged first, I'll update the code of the second PR. |
…oing a check Signed-off-by: jaudiger <[email protected]>
crates/brioche/src/check.rs
Outdated
| use std::process::ExitCode; | ||
| use std::vec; |
There was a problem hiding this comment.
| use std::vec; |
|
@kylewlacy do you prefer a rebase or a merge to resolve the merge conflicts ? |
Generally, I prefer merges specifically because GitHub's PR reviews work better with it (I can click "show changes since last review" and get a nice diff, but that tends to break when force pushing / rebasing). I always use "Squash and merge" when merging PRs anyway, so the final result in the |
Signed-off-by: jaudiger <[email protected]>
Signed-off-by: jaudiger <[email protected]>
Signed-off-by: jaudiger <[email protected]>
jaudiger
force-pushed
the
check-multiple-arguments
branch
from
e12668f
to
9a7244a
Compare
Sorry..... I didn't see when I was preparing the change locally... I'm used to rebase, since I like having work commit. But I'll now switch to the merge strategy for my other PRs! Basically, what I did was just to integrate the reporter stuff inside the three commits. If for the sake of review it doesn't suit you, please let me know. At least, for this time, the compare option seems to kind of work: https://github.com/brioche-dev/brioche/compare/e12668f5dc67886b70e1120773ea087deb9de9af..9a7244a230fcf59bb6402b04bc26778f5e7218cc (except I didn't touch the publish file) |
| ) -> Result<bool, anyhow::Error> { | ||
| let num_lockfiles_updated = projects.commit_dirty_lockfiles().await?; | ||
| if num_lockfiles_updated > 0 { | ||
| tracing::info!(num_lockfiles_updated, "updated lockfiles"); |
There was a problem hiding this comment.
One thing, I didn't touch this line, but should I?
There was a problem hiding this comment.
I think this is fine as-is, is there a reason it should be changed?
There was a problem hiding this comment.
No, I don't think so. I just wanted to raise the attention to this single call to tracing::info.
Not a problem 😄 It's basically just a personal preference, but more important is making sure it's a project where you and other contributors can use a workflow you're comfortable with first and foremost (which is also why I always go with "Squash and merge" when merging PRs into main, it seems like the middle ground between folks that like "merge" vs. "rebase", keeping the history clean while still including the commit messages and a link to the PR) |
Part of #75
I added the ability to set more than one registry and project when calling
brioche checkcommand. This is separated in two commits. The first updates the check command, while the second commit refactor the code of the format command to mimic what was done with the check.This has been done to extend more easily the next commands, cited in #75.