search

canonical / sphinx-docs-starter-pack

A documentation starter-pack
https://canonical-starter-pack.readthedocs-hosted.com/
Other
15 stars 40 forks source link

Improve Instructions for Local Checks #89

Closed ColmBhandal closed 1 year ago

ColmBhandal commented 1 year ago

Add instructions to users asking them to run spelling, link check and woke checks locally before committing, to reduce review churn.

Perhaps we could add a new make target that checks all 3?

lblythen commented 1 year ago

Perhaps we could add a new make target that checks all 3?

+1 IDK about other projects but KernOS, Kerncraft and Kernel Factory PRs can't be merged unless they pass all tests. That was mandated before my time but I support it fully. At some stage each PR has to be clean enough to pass them, so IMO get them clean at the start and avoid the churn.

ru-fu commented 1 year ago

so IMO get them clean at the start and avoid the churn.

Well, that's personal preference - I prefer to let the checks run on GitHub in a couple of minutes instead of 20+ minutes on my local machine (where I can't switch to another branch while they are running).

But sure, instructing users how to run them locally is definitely a good idea, and I'm fine with having a target for all of them as well (but we'd need to make sure that target then gives useful output and doesn't make you scroll back forever to see what went wrong).

ColmBhandal commented 1 year ago

OK, if it takes 20+ mins to run them all then that's a problem. I haven't tried running anything other than spell check on my machine.

So what about we do this. Expose all 3 commands in the readme - tell the user how to run them. Then, strongly recommend at least run spell check before pushing. Then, tell them if they like, they can run all 3 together in a single make target. And provide that make target.

lblythen commented 1 year ago

Yes, I'd back that. It could cut down the noise from new spelling issues from one push to the next, and IME spelling and woke are quick locally. For me it's linkcheck that takes ages. Once it's been run and the links fixed, unless they're changed or new ones are added there's little need to run it again until a final pre-merge check.

ru-fu commented 1 year ago

Sounds good to me.

And the time it takes depends on the number of files and the general time it takes to build the docs. For LXD, it's actually the spelling check that takes longest. But as long as nobody forces me to run it before pushing, I can ignore the advice and just get my spelling right without checks. ;)

ColmBhandal commented 1 year ago

OK I'll work on this one.

syncronize-issues-to-jira[bot] commented 1 year ago

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/DOCPR-84.

This message was autogenerated

ColmBhandal commented 1 year ago

OK I put up a PR. I didn't add a new make target because it looks like different people will want to mix and match different checks, depending on their setup. Instead, I just gave instructions on the existing make targets that would be useful for the user to run.