Skip to content

Refactor env_preferences to host_info #7113

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Refactor env_preferences to host_info #7113

wants to merge 2 commits into from
+3,355 −1,646

Conversation

zbraniecki
Copy link
Member

Depends on #7112 .
Partially resolves #5869 implementing refactor plus documentation, and fixing #6574 .

@zbraniecki zbraniecki requested review from a team and nciric as code owners October 16, 2025 15:06
@zbraniecki zbraniecki force-pushed the host_info branch 10 times, most recently from 7b7864e to 84e83fe Compare October 17, 2025 00:24
Manishearth
Manishearth reviewed Oct 17, 2025
View reviewed changes
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API overall looks quite straightforward

@zbraniecki
Copy link
Member Author

I'll need help figuring out how to avoid enabling gnome in our CI/CD at least. Right now I tried diabling it in Cargo.toml all-features section but it still gets enabled in many CI/CD commands. :(

robertbastian
robertbastian reviewed Oct 20, 2025
View reviewed changes
utils/host_info/src/lib.rs
//! | Date format | ⚠️ | ⚠️ | ⚠️ | ⚠️ | ⚠️ |
//! | Number format | ⚠️ | ⚠️ | ⚠️ | ⚠️ | ⚠️ |
//!
//! <sup>(1)</sup> In case of Linux different DE's such as Gnoem and KDE are supported together.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's a "DE"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Desktop Environment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please spell out. also Gnome has a typo

@dpulls
Copy link

dpulls bot commented Oct 22, 2025

🎉 All dependencies have been resolved !

@zbraniecki
Copy link
Member Author

Thanks all. Applied feedback and rebased on top of main. I'm still not sure how to handle working around --all-features trying to pull gio on Linux.

robertbastian
robertbastian reviewed Oct 22, 2025
View reviewed changes
Copy link
Member

@robertbastian robertbastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, I also don't know how to fix the --all-features test. Does the gio provide any guidance for use in libraries?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robertbastian robertbastian robertbastian left review comments

@nciric nciric Awaiting requested review from nciric

@Manishearth Manishearth Awaiting requested review from Manishearth

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Publish env_preferences?

3 participants

@zbraniecki @Manishearth @robertbastian