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

Create run-locally.sh to automate running locally, fix last commit, add name to Lint workflow #40

Closed
wants to merge 6 commits into from

Conversation

hahayupgit
Copy link
Member

@hahayupgit hahayupgit commented May 15, 2024

Enables users to run locally with 1 script. Script MUST be ran from .../whisky-book, or will not work. Script can be ran either ./scripts/run-locally.sh to generate the sidebar, check linting, and open the page, or ./scripts/run-locally.sh -i to install the cargo dependencies, and then do the same thing as ./scripts/run-locally.sh.

Also fixes the repository that mdbook-last-changed was pointing to, as it was previously the Whisky repo.

Add name Check Linting to linting workflow

@hahayupgit hahayupgit changed the title Create run-locally.sh to automate running locally Create run-locally.sh to automate running locally, fix last commit May 15, 2024
fix repo linked at bottom of pages
@hahayupgit hahayupgit changed the title Create run-locally.sh to automate running locally, fix last commit Create run-locally.sh to automate running locally, fix last commit, add name to Lint workflow May 15, 2024
@JoshuaBrest
Copy link
Contributor

I'm not really happy with these type of scripts. Here's why:

  1. Script should never install packages for the user even if they user ask for it.
  2. You shouldn't be checking if the base path is equal to whisky-book. This is because the user can clone into different directories.
  3. The script is kind of unnecessary. There's not really a purpose of running both generate and the lint tool at the same time it gives us a false sense of security that the sidebar is automatically generated.

@hahayupgit
Copy link
Member Author

Why shouldn't scripts install packages even if they ask for it? Genuinely curious.

I check if the base path is equal to whisky-book as mdbook serve --open only works in the root directory of whisky-book.

I personally like the script as I prefer one command to both generate the sidebar and open the book, though that's just me. I'm curious what you mean by "false sense of security" though. Would the generate.mjs script not throw errors if it was improperly generated?

@hahayupgit
Copy link
Member Author

Talked on Discord, I'm gonna close this for now and open another with the smaller changes, and re-implement the script later!

@hahayupgit hahayupgit closed this May 16, 2024
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.

2 participants