Fix cfgman documentation release #37
Conversation
... broken since release process moved to github releases.
yadij
added
the
S-waiting-for-PR
|
Requires squid-cache/squid#2257 to have been merged to both v7 and v8 for this to work. |
| git checkout v$version && | ||
| ./bootstrap.sh && ./configure && make -C ./doc cfgman && | ||
| mv -f -t $SQUID_WWW_PATH/content/Versions/$version/cfgman ./doc/cfgman/* |
There was a problem hiding this comment.
AFAICT, this particular command sequence ignores failures and lets the current iteration proceed to the "update DYN documents" step that, AFAICT, depends on the failed command. If we do not want to abort the script on failures, should not we proceed to the next iteration instead?
| gitCleanWorkspace () | ||
| { | ||
| git clean --quiet -xdf --exclude="\.BASE" | ||
| git checkout --quiet -- . |
There was a problem hiding this comment.
A checkout command does not belong to a "clean workspace" function. The command itself looks a bit odd.
If you want to erase changes to tracked files, then use something like git reset --hard. If tracked files may remain as is, then remove this command (from this function). Let the caller checkout what they need (one caller already does, resulting in two sequential checkouts).
| # Update the website /Versions/v*/cfgman/ HTML documents | ||
| cd $SQUID_VCS_PATH/squid-$version || continue | ||
| gitCleanWorkspace | ||
| git checkout v$version && |
There was a problem hiding this comment.
If this script relies on something else fetching changes from the remote repository, I recommend adding a comment about it. Otherwise, it needs to do something like "git fetch".
| gitCleanWorkspace | ||
|
|
||
| # Update the website /Doc/config/ DYN documents | ||
| ! test -d $SQUID_WWW_PATH/content/Versions/$version/cfgman && continue |
There was a problem hiding this comment.
It is strange to check whether this directory is present when the above commands rely on its presence to store their result. Should not this test be moved higher, where it used to reside before this PR?
| mv -f -t $SQUID_WWW_PATH/content/Versions/$version/cfgman ./doc/cfgman/* | ||
| gitCleanWorkspace | ||
|
|
||
| # Update the website /Doc/config/ DYN documents |
There was a problem hiding this comment.
Does this update use some make-generated files? If yes, we should not clean the working directory before this update (and we should not update if make fails). If not, I recommend rephrasing this comment to clarify where the new information is coming from, to remove the current implication that this information is the result of the above make.
| # Known Bug: | ||
| # Does not remove references to directives dropped. This is | ||
| # an issue for branch 'master' directive erasure. |
There was a problem hiding this comment.
Is not this bug still present? AFAICT, the DYN part of the loop iterates previously published *.html directive files and does not erase any stale ones.
rousskov
added
the
S-waiting-for-author
... broken since release process moved to github releases.
squid-anubis
added
the
M-waiting-staging-checks
... broken since release process moved to github releases.