Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: notify-rust now a mandatory dep #4172

Closed
wants to merge 1 commit into from
Closed

Conversation

zeenix
Copy link

@zeenix zeenix commented Jul 13, 2022

The reason to keep this an optional dep was a portability issue that was fixed in Feb:

NixOS/nixpkgs#160876

Motivation and Context

tbh, as the author of zbus crate, I was happy to see that starship depeneds on it indirectly on Linux. This change would mean a mandatory dep on it (even if indirectly).

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

I've only checked if the build works but given that that notify-rust was default, I seriously doubt this can break anything.

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

git grep notify-rust didn't reveal any docs or tests to update. I did ensure tests build and run successfully though.

The reason to keep this an optional dep was a portability issue that was
fixed in Feb:

NixOS/nixpkgs#160876
@davidkna
Copy link
Member

Have you confirmed that the issue was fixed? NixOS/nixpkgs#160876 was closed because notify-rust was made optional in starship again.

@zeenix
Copy link
Author

zeenix commented Jul 13, 2022

@davidkna Thanks for the quick response!

Have you confirmed that the issue was fixed? NixOS/nixpkgs#160876 was closed because notify-rust was made optional in starship again.

Of course I didn't. That would have made sense. :) Sorry, I was still on my first cup of tea and have jetlag. :)

@zeenix zeenix closed this Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants