feat: update the options generation to be more general

[taciturnaxolotl]

• I have read CONTRIBUTING.md.
• I have created a discussion that was approved by a maintainer (for new features).

This will need to be merged into #33 with the actual update of the config. This is just the script update

[malikwirin]

Have you run a linter on that python file?

[taciturnaxolotl]

not yet; what linter would you prefer me to use?

[taciturnaxolotl]

I ran ruff on it and it didn't find anything while linting, but it did reformat it.

[malikwirin]

not yet; what linter would you prefer me to use?

Black is somewhat of the standard in the Nix community, but I personally also prefer ruff even though without configuration it is not as strict

I was asking because of a dislike for variables and function executions in global namespace. Those things fit better in a main function

[taciturnaxolotl]

sweet! I'll run it through black too then; I don't write much python except for random one off scripts so thats quite helpful :)

[taciturnaxolotl]

how does that look?

[malikwirin]

how does that look?

Does looks way better. I can properly review it tomorrow.
The script is for generating and updating the settings.nix based on the actual options provided by crush?

[taciturnaxolotl]

it is! It uses the JSON schema

[taciturnaxolotl]

I probably should modify the script to make a check the current crush version first and pull the proper version from GitHub instead of just using the latest schema

[malikwirin]

I will try out the script on top of #33. How is one supposed to use it? Can you add a README.md for the scripts/ directory?

[taciturnaxolotl]

Done! Just run it like ./scripts/generate-options.sh > modules/crush/options.nix

[malikwirin]

Done! Just run it like ./scripts/generate-options.sh > modules/crush/options.nix

This wont work if it is actually supposed to build upon #33 as I renamed modules/crush/options.nix to modules/crush/options/settings.nix to seperate the configuration options from nix module typical options like package

If this PR is supposed to be able to be merged before #33 I can fix that in my PR should it happen

[taciturnaxolotl]

I was originally thinking this would be merged first but it probably makes more sense to merge after yours

[malikwirin]

I was originally thinking this would be merged first but it probably makes more sense to merge after yours

Its not up to me, but I would suggest changes that could be merged at any time. Depending on the merge order we can fix the file and path naming easily. options --> settings

[taciturnaxolotl]

yeah i'll keep it like this till we need it then

[taciturnaxolotl]

Good to merge @aymanbagabas! I'll look at trying to trigger that script whenever crush updates

[taciturnaxolotl]

as per discussion on the slack I moved the script over to using go generator and added a github action

[malikwirin]

Maybe rename the script to generate-crush-settings.go as well. Because of the path of the script its not obvious that it would only generate the setting of crush. (I know currently it is the only module but still)

[taciturnaxolotl]

should be good to go!

[malikwirin]

Sorry if this is bothering but I still am kinda unhappy with the naming. The name of the script implies that it generates the options of crush. But it only generates the options related to the configuration that without the module happens in the ~/.config directory.

[taciturnaxolotl]

oh fair lol

[taciturnaxolotl]

renamed to workflow to follow the same convention as well

[malikwirin]

LGTM