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

KISS build docs #102

Open
wants to merge 3 commits into
base: freeze-feat-andreas
Choose a base branch
from

Conversation

murlakatamenka
Copy link

Fixes #101

  • makes iterations while working on wayshot faster, thus better for devs and contributors.
  • makes make more explicit about what it does
  • fixes a minor issue of docs/*.7.gz not being cleaned

Fixes waycrate#101

Why?

- no build.rs that is invoked on _any_ `cargo build/run`
- no extra dev dependency
- no intermediate `docs/*.out` files
- 3 lines of bash instead
- maximum gzip compression (-9/--best)
- docs will be built only when needed,
    i.e. on `make docs` or `make install`

To quickly check:

```sh
make TARGET_DIR=/tmp/wayshot MAN{1,7}_DIR=/tmp/wayshot install
```
Shows exactly what is going on.

Also fixes a minor issue of not cleaning `./docs/*.7.gz`
@Shinyzenith
Copy link
Member

Hi @murlakatamenka. This needs to be rebased onto freeze-feat-andreas branch

@Shinyzenith
Copy link
Member

Make check earlier used to ensure that scdoc could compile the docs too. We use it in CI.

if that can be implemented here then we can go ahead with a merge.

@murlakatamenka murlakatamenka changed the base branch from main to freeze-feat-andreas March 14, 2024 22:31
@murlakatamenka
Copy link
Author

murlakatamenka commented Mar 14, 2024

Make check earlier used to ensure that scdoc could compile the docs too. We use it in CI.

if that can be implemented here then we can go ahead with a merge.

Previously the docs were built into gzipped man pages during cargo build/build.rs:

cargo build --release

And also checked in a separate job:

documentation:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Check docs
run: |
sudo apt update
sudo apt install --no-install-recommends scdoc
for file in $(find . -type f -iwholename "./docs/*.scd"); do scdoc < $file > /dev/null; done


Now that build.rs is gone, only documentation job checks if scdoc can process the files, this should be enough for CI. Gzipping the output can be considered infallible.

Ideally there should be a job or workflow that gets triggered only if those *.scd files change, because such changes are rare compared to modifying the sources.


So far I'm done with the changes, feel free to review.

@Shinyzenith
Copy link
Member

Things look great. Thanks for the PR!

@Shinyzenith
Copy link
Member

I just realized something I forgot to ask. The code you've written is POSIX compliant right? I would rather keep away from bash extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

build.rs is misused and invoked on any change to sources
2 participants