canonical / sphinx-docs-starter-pack

A documentation starter-pack
https://canonical-starter-pack.readthedocs-hosted.com/
10 stars 30 forks source link

Don't globally install software from Makefile #244

Open samuelallan72 opened 1 month ago

samuelallan72 commented 1 month ago

The makefile currently checks for woke, and if not found it installs it system wide with sudo snap install:

https://github.com/canonical/sphinx-docs-starter-pack/blob/0efecfd9d530bff8c173155a796f47e487eb95c0/Makefile.sp#L46

A makefile shouldn't try silently installing something system-wide with root privileges. Could I suggest it bail out with a message about needing to install woke instead? It could link to the source code and suggest sudo snap install woke to install.

ru-fu commented 1 month ago

Could I suggest it bail out with a message about needing to install woke instead? It could link to the source code and suggest sudo snap install woke to install.

Unfortunately, that won't work for running the check on GitHub - there we need to install it. But we could probably have two different targets, one that installs and one that prompts?

samuelallan72 commented 1 month ago

Couldn't it be installed in the github workflow before running the make target? :)

ru-fu commented 1 month ago

Yes, adding it to https://github.com/canonical/documentation-workflows/blob/main/inclusive-language/action.yaml would probably also be an option (but it would only work for teams that use the shared workflow).