-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
[refactor] environment variable initialization #243
Conversation
@Shinyzenith I am tagging you as you asked :) |
Hi @newtoallofthis123 from a preliminary view, things look great as we discussed. Can we expand this for swhkd too? If you'd like to impl that in a separate pr then let me know! |
Yes. This PR mostly aims to refactor the swhks code. I was planning on opening a similar PR for swhkd once this got approved, however, if need be I can combine it with this. However, a separate PR would be ideal I feel. Thanks for the review 😊 |
Cool, we can do that separately, let me just get zubairs opinion on this too before I merge. CC: @zubairmh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good pr, I've listed some minor changes you can incorporate
Hi it seems you also need to format your code |
Done! Just ran Hopefully that resolves the workflows 😄 and I also removed the |
This PR implements the following changes, these changes have been limited to
swhks
for feedback and review, but they can be extended into the actualswhkd
as well.1. Env Struct
The env struct is used to centrally handle all the environment related queries and initializations. It is opened as a thread safe
OnceLock
variable and hence is ideal for servers2. Better Error Handling
Error handling related to env is now centrally handled in the Env struct itself using typecasted enumerations
3. PathBuf implementations
Paths are now handled with
pathbufs
instead of format stringsThis PR lays the ground work in centralizing the Environment variable queries that would help abstract away all the individual queries. This could especially be helpful when we modify the privilege model since we would now need to do changes in only a few places
Additionally, this has been tested individually and cargo linted as well
Feedback and any changes required are appreciated 😄