83 setup script #85
83 setup script #85
Conversation
The file takes the working dir as input Then it copies and updates files as necessary
readme.rst
Outdated
|
|
||
| Getting started | ||
| --------------- | ||
| Local Development |
There was a problem hiding this comment.
I felt that it better describes the content, which describes how to build and run and develop locally.
Also, the user may not only want to consult this when getting started.
Also, the user has already god started by running the bootstrap so it felt a bit weird telling them to get started mid doc.
There was a problem hiding this comment.
OK, I see the point about "Getting started" not being good, but I don't like the "local development". At this time, we don't have anything else (RTD hasn't been set up, and we want everybody to build their docs locally first before doing anything else). So maybe just "Development" or "Start working" or "Build and view the docs" or something?
There was a problem hiding this comment.
Good point. I changed to "Build and view the docs".
readme.rst
Outdated
| .. code-block:: none | ||
| * Copy [bootstrap.sh](bootstrap.sh) to your repository's root directory. | ||
| * Run the script: ``./boostrap.sh`` | ||
| * Enter the installation directory when prompted. For standalone repos, enter ".". For docs alonside code, enter "docs". |
There was a problem hiding this comment.
| * Enter the installation directory when prompted. For standalone repos, enter ".". For docs alonside code, enter "docs". | |
| * Enter the installation directory when prompted. For standalone repositories, enter ".". For documentation alongside code, enter "docs". |
There's a few more cases of "repo" and "docs", which we shouldn't use imo. They'll also break the spelling check.
readme.rst
Outdated
|
|
||
| * ``configuration: docs/conf.py`` | ||
| * ``requirements: docs/.sphinx/requirements.txt`` | ||
| When the script completes, your repository will have the starter sphinx docs files added. Commit your changes. |
There was a problem hiding this comment.
We should add a summary of what the script does somewhere in this section. You can't expect anybody to want to run a script that copies files around without telling them what the script will do. ;)
There was a problem hiding this comment.
Added a description.
readme.rst
Outdated
|
|
||
| * ``configuration: docs/conf.py`` | ||
| * ``requirements: docs/.sphinx/requirements.txt`` | ||
| When the script completes, your repository will have the starter sphinx docs files added. Commit your changes. |
There was a problem hiding this comment.
| When the script completes, your repository will have the starter sphinx docs files added. Commit your changes. | |
| When the script completes, your repository will have the files added that are needed to get started with Sphinx documentation. Commit your changes. |
readme.rst
Outdated
|
|
||
| Getting started | ||
| --------------- | ||
| Local Development |
bootstrap.sh
Outdated
|
|
||
| # Update working-directory field in workflow files | ||
| echo "Updating working directory in workflow files..." | ||
| sed -i "s|working-directory: .*|working-directory: $install_directory|g" .github/workflows/* |
There was a problem hiding this comment.
NO! This will change all working directories in existing workflows (which might not be related to documentation).
Either change this before moving the workflow out of the temp directory, or restrict to our workflow.
There was a problem hiding this comment.
Also, we know what our workflow file contains, so why use a wildcard?
There was a problem hiding this comment.
Good catch. I've updated the sed to run much earlier in the script. I left it as a wildcard in case we add more workflows.
bootstrap.sh
Outdated
|
|
||
| # Update .readthedocs.yaml configuration | ||
| echo "Updating .readthedocs.yaml configuration..." | ||
| sed -i "s|configuration: .*|configuration: $install_directory/conf.py|g" "$install_directory/.readthedocs.yaml" |
There was a problem hiding this comment.
Again, why use a wildcard for matching? We know what we want to replace, and if the content is different, we don't want to blindly replace.
There was a problem hiding this comment.
Yep Lloyd had a similar comment and now I only match conf.py
Only replace configuration ending with conf.py This prevents overwriting other configuraitons
Co-authored-by: Ruth Fuchss <ruth.fuchss@canonical.com>
Docs and repos Replaced with full words
Running sed straight after clone This ensures it's only run against starter pack And it doesn't affect their existing repo
bootstrap.sh
Outdated
| echo "Updating working directory in workflow files..." | ||
| sed -i "s|working-directory: .*|working-directory: $install_directory|g" "temp-starter-pack/.github/workflows"/* | ||
| echo "Updating .readthedocs.yaml configuration..." | ||
| sed -i "s|configuration: .*conf\.py|configuration: $install_directory/conf.py|g" "temp-starter-pack/.readthedocs.yaml" |
There was a problem hiding this comment.
.*conf\.py ?
I think we should either use exactly what we have in the readthedocs.yaml file (conf\.py, similar in the next line) or wildcards. Mixing the two doesn't make any sense imo.
There was a problem hiding this comment.
Yeah makes sense. I left a whitespace wildcard in there but other than that made it strict as you said.
bootstrap.sh
Outdated
| echo "Cleaning up..." | ||
| rm -rf temp-starter-pack | ||
|
|
||
| echo "Setup completed!" |
There was a problem hiding this comment.
Can you add a line break at the end?
readme.rst
Outdated
| * Copies contents and workflow files from the starter pack to the installation directory | ||
| * Udates working directory paths in workflow files, and updates configuration paths in the .readthedocs.yaml file |
There was a problem hiding this comment.
These are now switched around.
Co-authored-by: Ruth Fuchss <ruth.fuchss@canonical.com>
The description steps need to be reordered The order in the script has changed
SSH will only work for those with it set up
|
Why was this merged if the tests didn't pass? |
I have no idea what happened. I swear I didn't click the merge button, but the above seems to prove otherwise... anyway, Ruth reverted the commit and force pushed main very quickly afterwards so should be all good. |
Closes #83.
This is quick and dirty. The user just copies the bash script into their repo and runs it, and the script clones & sets up the files for them.
Please take a note of the readme changes I made.