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

modules: add platformOptimizations #172

Merged
merged 8 commits into from
Mar 29, 2024
Merged

Conversation

RTUnreal
Copy link
Contributor

These settings are taken from the steamos-customizations-jupiter package on SteamOS.

@fufexan
Copy link
Owner

fufexan commented Mar 28, 2024

Is this supposed to be used with https://github.com/Jovian-Experiments/Jovian-NixOS?

@RTUnreal
Copy link
Contributor Author

No, it is just a standalone config for settings, some people might want to use

@fufexan
Copy link
Owner

fufexan commented Mar 28, 2024

Perhaps you could also add a section for the module in the readme, in the same manner as for pipewire low-latency.

@NotAShelf
Copy link
Contributor

NotAShelf commented Mar 28, 2024

I'm not sure if I like the module name. Much rather give it a blanket name (e.g. programs.steam.platformOptimizations) that can be extended later on.

Also, I believe the vm map count is specific/optimized for x86_64 platforms so that might be worth setting based on host platform.

@RTUnreal
Copy link
Contributor Author

I'm not sure if I like the module name. Much rather give it a blanket name (e.g. programs.steam.platformOptimizations) that can be extende later on.

Also, I believe the vm map count is specific/optimized for x86_64 platforms so that might be worth setting based on host platform.

I am not picky about the name. If you have a better suggestion, then please share it.

@RTUnreal
Copy link
Contributor Author

But also after searching for a bit, It is not necesary as, like the comment says, the kernel uses an int. And from what I can tell, on all NixOS platforms int = 32bit integer

@fufexan
Copy link
Owner

fufexan commented Mar 28, 2024

I'm also in favor of renaming the option to programs.steam.platformOptimizations.enable, like raf said.

@RTUnreal RTUnreal changed the title modules: add steamosSysctls modules: add platformOptimizations Mar 29, 2024
Copy link
Owner

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

LGTM

@NotAShelf do you have anything to add?

@NotAShelf
Copy link
Contributor

NotAShelf commented Mar 29, 2024

I could nit the enable option description, but really not worth the trouble. LGTM.

@fufexan fufexan merged commit 350a573 into fufexan:master Mar 29, 2024
6 of 8 checks passed
@RTUnreal
Copy link
Contributor Author

If it is described too confusingly, then go ahead and nit it. Descriptions are meant to be descriptive. 👍

@RTUnreal RTUnreal deleted the steamos-sysctls branch March 29, 2024 16:24
@NotAShelf
Copy link
Contributor

Nothing to do with descriptiveness. It's good enough, I'm just worried about future-proofing - e.g. we might want to add alternative/extra lines that are not set in SteamOS, but that's not too big of a deal. Worst case, we'll redo the description in the future.

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.

3 participants