No 'failed step' notification

r-darwish / topgrade

Upgrade everything

GNU General Public License v3.0

3.36k stars 161 forks source link

No 'failed step' notification #917

Closed r-darwish closed 2 years ago

r-darwish commented 2 years ago

Fixes #903

Standards checklist:

[ ] The PR title is descriptive.
[ ] The code compiles (cargo build)
[ ] The code passes rustfmt (cargo fmt)
[ ] The code passes clippy (cargo clippy)
[ ] The code passes tests (cargo test)
[ ] Optional: I have tested the code myself
- [ ] I also tested that Topgrade skips the step where needed

If you developed a feature or a bug fix for someone else and you do not have the means to test it, please tag this person here.

r-darwish commented 2 years ago

@JacobTravers can you please test this branch?

jtrv commented 2 years ago

I tested cargo build, cargo fmt, cargo clippy, and cargo test all appear to pass, and the step-fail notificiations appear to be working as well! 👍🏽 Thanks again for looking into this!

jtrv commented 2 years ago

Additionally I think it would be great to add 'critical' urgency to the fail notifications. I could try later this week when I have the time, it seems easy enough and I've been trying to find small rust practice projects. Here is the relevant part of notify-rust if you would like to knock it out.

https://docs.rs/crate/notify-rust/latest/source/src/urgency.rs

r-darwish commented 2 years ago

Last time I checked the notifications crate needed a dynamic C library. I'll merge this as it fixes the bug and later on I'll check if there's a rust native notification crate