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

[refactor] environment variable initialization #243

Merged
merged 6 commits into from
Feb 27, 2024

Conversation

newtoallofthis123
Copy link
Contributor

@newtoallofthis123 newtoallofthis123 commented Feb 26, 2024

This PR implements the following changes, these changes have been limited to swhks for feedback and review, but they can be extended into the actual swhkd 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 servers

2. 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 strings

This 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 😄

@newtoallofthis123 newtoallofthis123 changed the title Refactor file path handling and environment variable initialization Refactor environment variable initialization Feb 26, 2024
@newtoallofthis123 newtoallofthis123 changed the title Refactor environment variable initialization [refactor] environment variable initialization Feb 26, 2024
@newtoallofthis123
Copy link
Contributor Author

newtoallofthis123 commented Feb 26, 2024

@Shinyzenith I am tagging you as you asked :)

@Shinyzenith
Copy link
Member

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!

@newtoallofthis123
Copy link
Contributor Author

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 😊

@Shinyzenith
Copy link
Member

Cool, we can do that separately, let me just get zubairs opinion on this too before I merge.

CC: @zubairmh

@Shinyzenith Shinyzenith added the Enhancement New feature or request label Feb 27, 2024
Copy link

@zubairmh zubairmh left a 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

.gitignore Outdated Show resolved Hide resolved
swhks/src/environ.rs Show resolved Hide resolved
@Shinyzenith
Copy link
Member

Hi it seems you also need to format your code make check.

@newtoallofthis123
Copy link
Contributor Author

newtoallofthis123 commented Feb 27, 2024

Done! Just ran make check

Hopefully that resolves the workflows 😄 and I also removed the .gitignore commit

@Shinyzenith Shinyzenith merged commit 994b74f into waycrate:main Feb 27, 2024
7 checks passed
@Shinyzenith Shinyzenith linked an issue Feb 27, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Proposal: Better Path handling Mechanisms
4 participants