-
Notifications
You must be signed in to change notification settings - Fork 54
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
Rose user guide: versioning #2190
Rose user guide: versioning #2190
Conversation
* Changed docs structure to $ROSE_HOME/doc/$VERSION/$FORMAT * Added support for Jekyll into the repository * HTML static redirects now generated on build * New version selector for html documentation (ws only)
e2e1fce
to
24adf31
Compare
24adf31
to
645bf9a
Compare
|
||
## 404 Page Not Found - The Rose Documentation Has Moved | ||
|
||
Please update your bookmarks to: http://metomi.github.io/rose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could write this file in rose make-docs
if we don't want it kicking about in the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine as it is.
"_modules", | ||
"_sources", | ||
"_static" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same applies to the Jekyll config too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't find anything to pick on!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The versioning concept outlined is very good & implemented fine. Builds (in virtual env. required) fine to the structure as described. I have a few minor comments.
lib/python/rose/resource.py
Outdated
version = os.getenv("ROSE_VERSION") | ||
if version is None: | ||
def get_version(self, ignore_environment=False): | ||
"""Return ROSE_VERSION. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider padding this description out to explain that this is (from the environment variable) otherwise as set in rose-version
; while obvious from the code, the standalone docstring, as possibly auto-parsed to docs, would be clearer with some context.
} | ||
|
||
version_file () { | ||
# output the dictionary {"version": ["build", ...]} in JSON format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would usually advise using (perhaps embedded) Python (or another language, but this in our context) instead of bash to do anything this complex, & especially relating to JSON, but this seems to work fine and isn't too contrived.
|
||
# commit and push | ||
git commit -m "${ROSE_VERSION}" | ||
# git push "${REMOTE}" gh-pages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has the push
line (both here & in the subsequent_deployment
function) been commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At present the deploy-docs script serves only as a guide. Up until the git push
the script has only local effects, after this is has the ability to modify the live documentation.
In a future pull we will look at auto-deployment and will have to work towards removing these comments (for non-interactive usage), we will need good confidence in the script!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspected it was some sort of safety measure like this. Thanks for explaining.
rm -rf "${VENV_PATH}" | ||
} | ||
# Path for virtualenv. | ||
VENV_PATH='venv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would "${ROSE_HOME}/venv"
perhaps be safer here, or is there a reason to use the basename only (I can't think of any)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Towards the top of the file we do:
cd "$(dirname "$0")/../"
so we are already in "${ROSE_HOME}".
(Re-approved after final commit which is trivial hence fine to merge PR without first reviewer re-approval.) |
Add functionality to
rose make-docs
to build versioned documentation.Documentation is built incrementally so new versions can be added without having to rebuild old ones i.e:
As the documentation is built incrementally it is non-destructive in version control terms meaning we can do:
$ git push upstream gh-pages # no need to -f force
This would make auto-deployment of documentation much nicer.
Static HTML redirects are created by
rose make-docs
including a legacy re-direct fordoc/rose.html
.The version switcher AJAXes the versions.json file to get a dictionary of available versions and formats, this AJAX means that the version selector will only work when served from behind a web server (or very permissive browser - cross site scripting).