<cole-h>
OK, I'm slowly warming up to the idea of sending the diffstat to /dev/null instead of just saying `--no-patch`...
<gchristensen>
sgtm
orivej has quit [Ping timeout: 240 seconds]
cole-h has quit [Quit: Goodbye]
orivej has joined #nixos-borg
orivej has quit [Ping timeout: 256 seconds]
cole-h has joined #nixos-borg
cole-h has quit [Ping timeout: 256 seconds]
cole-h has joined #nixos-borg
cole-h has quit [Client Quit]
cole-h has joined #nixos-borg
<cole-h>
Well, looks like ofborg didn't choke on the RFC45 PR this time. Good work gchristensen :^)
<gchristensen>
woot
<gchristensen>
thank you!
* cole-h
closes the Loki tab looking for internal errors
* cole-h
leaves the GitHub search for the internal error label open, though
<cole-h>
gchristensen: I'm thinking of setting stdout AND stderr to null on all `status`'d commands (not ones that use `output` to check stdout/err). Are there any that produce useful info that I shouldn't do this to?
<cole-h>
(more logging cleanup)
<gchristensen>
hrm
<gchristensen>
not sure, which ones are you thinking?
<cole-h>
The ones in src/clone.rs and src/checkout.rs: `clone_repo`, `fetch_repo`, `clean`, `checkout` in clone.rs and `fetch_pr`, `commit_exists`, `merge_commit` in checkout.rs
<cole-h>
(NOT `commit_messages_from_head` and `files_changed_from_head`, which don't output to stdout/err, but consume it)
<cole-h>
(in checkout.rs
<cole-h>
)
<gchristensen>
sgtm
<gchristensen>
btw if you want to delete all the locking code and replace it with a single process-wide lock, that'd be cool :)
<cole-h>
If something breaks and doesn't get logged because of this, feel free to yell at me :P
<cole-h>
Hmm, unsure how I would/should go about that part
<gchristensen>
no worries
<cole-h>
Would you expect that locking to go in the actual binary (bin/*.rs file), or were you thinking of something different
<gchristensen>
not sure
<cole-h>
Or maybe the tasks/*.rs
<gchristensen>
maybe the bin/*.rs
<LnL>
the output is sometimes useful, but mostly because the pr that's building is a bit hard to find among everything else
<gchristensen>
I just know the currently locking doesn't do what it should do, because the locks don't hold long enough
<gchristensen>
LnL: I'm sort of hoping that adopting Slog will let each log message include context about what PR its working on
<LnL>
yeah
<LnL>
including the pr in eg. the can build / can't build message would totally solve this :)
<gchristensen>
yeah :)
<gchristensen>
now to just do the leg work of adding slog :P
<cole-h>
Hehe
<LnL>
and something more structured would definitively be useful if the builders where ever to do multiple things at the same time
<gchristensen>
+1000
<cole-h>
+10000
<gchristensen>
LnL: I've sort of considered replacing the builder model with a build farm like h.n.o and a couple machines running nix-buidl on the farm
<MichaelRaskin>
Indeed, now that you don't want heterogenously managed builders…
<LnL>
right, guess it depends what direction you want to go
<cole-h>
LnL: the function that reports can/can't build is provided with a `BuildJob`, which has a `Pr`, which has the PR number... Sounds doable :)
<MichaelRaskin>
Maybe include commit id…
<gchristensen>
don't do that :P preferrable to go the context route that string interplotaion eerywhere
<MichaelRaskin>
(because I have seen some complicated ordering of events inside some PR discussions)
<cole-h>
Don't do which, commit ID, or add PR number to "can/can't build log"?