Skip to content

Make building the package docs more convenient #2173

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Make building the package docs more convenient #2173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@ AbstractAlgebra = "c3fe647b-3220-5bb0-a1ea-a7954cac585d"
Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"

[compat]
Documenter = "1.0"
Documenter = "1"

[sources]
AbstractAlgebra = {path = ".."}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is an unambiguous win and can be merged.

35 changes: 34 additions & 1 deletion docs/make.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,36 @@
#
# Usage: from the package root directory, run
#
# julia --proj=docs docs/make.jl
#
# To fix doctests instead of merely running them, use
#
# DOCTEST=fix julia --proj=docs docs/make.jl
#
# To disable doctests, run
#
# DOCTEST=off julia --proj=docs docs/make.jl
#
using Pkg
Pkg.update()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is debatable I guess, and perhaps indeed clearly bad (as it may e.g. hinder CI). The main reason I put it in is because I feel forgetting to run update in your docs environment is a common cause of issues.

Perhaps it should be changed to instantiate or entirely dropped. If we drop it, I think we should at least put in language into the comment above that recommends running the update manually in that environment on occasion.

Or, we could keep it, but add a way to control it via a command argument and/or environment var, just like the doctest. (And what the default value would be could be debated).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the past, I used to dev the main package into the docs environment. This then did not need any updating of the docs environment at all. (will look more in-depth at this PR later)


# parse some options
doctest_arg = true
if "--fix" in ARGS
doctest_arg = :fix
else
tmp = get(ENV, "DOCTEST", "true")
Comment on lines +19 to +22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is overkill to use both an env variable and and a command line argument to turn on "fix" mode. Thing is, I started with the idea of adding a command line argument, and just --fix, but then realized I also want to be able to turn off the doctests entirely. And then I got a bit lazy, as I'd have to decide what the args should be named (--doctest=on/off/fix?) and how to parse it ;-).

I think overall I'd find a command line argument nicer, and would be willing to implement it properly, but before I spend effort on it, I'd like to find out what others think.

if tmp == "true" || tmp == "on" || tmp == "yes"
doctest_arg = true
elseif tmp == "false" || tmp == "off" || tmp == "no"
doctest_arg = false
elseif tmp == "fix"
doctest_arg = :fix
else
error("invalid DOCTEST env var")
end
end

using Documenter, AbstractAlgebra

DocMeta.setdocmeta!(AbstractAlgebra, :DocTestSetup, AbstractAlgebra.doctestsetup(); recursive = true)
Expand All @@ -10,7 +43,7 @@ makedocs(
sitename = "AbstractAlgebra.jl",
modules = [AbstractAlgebra],
clean = true,
doctest = true,
doctest = doctest_arg,
checkdocs = :none,
pages = [
"index.md",
Expand Down
Loading