Skip to content

Adding dropdown for versions#1046

Merged
chupman merged 1 commit intoJanusGraph:0.2from
chupman:version-dropdown
Aug 20, 2018
Merged

Adding dropdown for versions#1046
chupman merged 1 commit intoJanusGraph:0.2from
chupman:version-dropdown

Conversation

@chupman
Copy link
Member

@chupman chupman commented May 2, 2018

Signed-off-by: Chris Hupman chupman@us.ibm.com

I created a page with links to all the available documentation folders and added it to the appendices. I also replaced the other versions link that pointed to the releases page with a dropdown to switch between doc versions. If you click on Other Doc Versions it will take you to the new Appendix E, Other documentation versions.

Open issue is in the docs repo

Preview available here

Currently the changes can only be seen from the 0.3.0-SNAPSHOT or latest url. The changes will need to be backported into all the branches we plan to host documentation for. Also right now the version links are hardcoded in doc-versions.adoc and common.xsl. It would be easy enough to store them as an array in the pom, but you'd still have to update the pom for each individual release branch so it wouldn't really save much, or any, time.


Thank you for contributing to JanusGraph!

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there an issue associated with this PR? Is it referenced in the commit message?
  • Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
  • Has your PR been rebased against the latest commit within the target branch (typically master)?
  • Is your initial contribution a single, squashed commit?

For code changes:

  • Have you written and/or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository?
  • If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?
  • If this PR is a documentation-only change, have you added a [skip ci]
    tag to the first line of your commit message to avoid spending CPU cycles in
    Travis CI when no code, tests, or build configuration are modified?

Note:

Please ensure that once the PR is submitted, you check Travis CI for build issues and submit an update to your PR as soon as possible.

Copy link
Member

@mbrukman mbrukman left a comment

Choose a reason for hiding this comment

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

I would advocate for removing 0.2.0-SNAPSHOT since it wasn't an official release, and similarly suggest removing 0.3.0-SNAPSHOT (since once we release 0.3.0, we would want folks to read those docs, rather than the -SNAPSHOT version).

In general, I think of the *-SNAPSHOT docs as temporary while we don't have the release, but I don't want to hardcode it into any menus, just let folks access it if they know it's there.

Adding it here into the code means that the next version's docs will hard-code that version, and if we want to hide it, we have to rebuild the docs, which then violates the principle that the version is not modified after release without changing the version number...

Thoughts?

[appendix]
== Other documentation versions
* link:../0.3.0-SNAPSHOT/index.html[Version 0.3.0-SNAPSHOT]
* link:../0.2.0-SNAPSHOT/index.html[Version 0.2.0-SNAPSHOT]
Copy link
Member

Choose a reason for hiding this comment

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

This was done before the 0.2.0 release, but I would remove it since it's unreleased, and there's an official 0.2.0 release.

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 totally agree about removing all the snapshot links and just wanted them for PoC. I actually find the SNAPSHOT folders to be misleading since they're not updated on the live site after the release. Initially I expected them to be a more current version of the docs rather than a different 6 month old copy.

Since the docbook is generated html I'll need to push these changes to every branch we want listed, otherwise you'll only be able to see the doc-versions page and the dropdown from the 0.3.0 folder. I'm thinking it will be 1.0,1.1,2.0,2.1, and 3.0. Should I start with 1.0 and merge commit my way up?

Separately I want to work on dynamically updating the docs site with CI. Right now we're applying fixes to the docs in the 0.2.0 and most of them are for items that are misleading or completely incorrect. I know I've run into issues more than once because I was following the online docs only to find out that a PR addressing the issue had already been merged. Are you open providing the most current documentation on the live site rather than preserve consistency with the release?

<li class="header-item-right dropdown"><a href="doc-versions.html">Other Doc Versions</a>
<div class="dropdown-content">
<a href="../0.3.0-SNAPSHOT/index.html">Version 0.3.0</a>
<a href="../0.2.0-SNAPSHOT/index.html">Version 0.2.1</a>
Copy link
Member

Choose a reason for hiding this comment

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

Remove?

@chupman chupman force-pushed the version-dropdown branch 3 times, most recently from ce62353 to ddcad40 Compare May 3, 2018 17:31
@chupman
Copy link
Member Author

chupman commented May 3, 2018

After some additional digging this is how I think this should be implemented.

  1. We should have folders for updated docs for the 2 supported major releases generated by CI.
  2. Only the 0.2-latest and 0.3-latest doc folders will have the dropdown feature.
  3. The CI job that builds 0.2-latest and 0.3-latest will copy an updated version of doc-versions.html to all the release versions, 1.0,1.1,2.0, etc.
  4. All the release docs will need to be regenerated once but with the Other Versions link being pointed at doc-versions.html instead of the releases page.
  5. I'll retarget this for 0.2 branch, we can merge it forward to 0.3, and include links for 0.2.1 and 0.3.0 since the release is around the corner.

@mbrukman thoughs? Also do you want me to submit a PR to docs.janusgraph.org with the proposed rebuild of 0.1.0, 0.1.1, and 0.2.0?

@chupman chupman force-pushed the version-dropdown branch from ddcad40 to 74f0a35 Compare May 14, 2018 22:37
@chupman chupman changed the base branch from master to 0.2 May 14, 2018 23:51
@pluradj pluradj added this to the Release v0.2.1 milestone May 15, 2018
Copy link
Member

@pluradj pluradj left a comment

Choose a reason for hiding this comment

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

a couple thoughts

* link:../0.2.1/index.html[Version 0.2.1]
* link:../0.2.0/index.html[Version 0.2.0]
* link:../0.1.1/index.html[Version 0.1.1]
* link:../0.1.0/index.html[Version 0.1.0]
Copy link
Member

Choose a reason for hiding this comment

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

We currently package a zip file with the docs for every release. If a user has a local copy of the docs on their computer, none of these relative URLs would resolve. These should be linked to the docs website like https://docs.janusgraph.org/0.2.0/index.html

<a href="../0.2.0/index.html">Version 0.2.0</a>
<a href="../0.1.1/index.html">Version 0.1.1</a>
<a href="../0.1.0/index.html">Version 0.1.0</a>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Same comment on the relative URLs as above.

The dropdown is nifty, but as more releases go out, the number of items must be limited otherwise the list will become way too long to fit the dropdown in the browser window. Perhaps not a concern for right now, but it will need to be handled at some point.

@chupman chupman force-pushed the version-dropdown branch from 74f0a35 to 09a642f Compare May 16, 2018 22:55
@chupman
Copy link
Member Author

chupman commented May 16, 2018

@pluradj links are hardcoded now. As for the dropdown if we want to keep it terse restricting it to the supported releases would be the easiest method. People could still use the Other Doc versions link to get to the old stuff.

Copy link
Member

@pluradj pluradj left a comment

Choose a reason for hiding this comment

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

LGTM

@pluradj pluradj requested review from mbrukman, sjudeng and twilmes May 21, 2018 16:23
@janusgraph-bot janusgraph-bot added the cla: yes This PR is compliant with the CLA label May 22, 2018
@pluradj pluradj requested a review from a team May 30, 2018 15:07
[[doc-versions]]
[appendix]
== Other documentation versions
* http://docs.janusgraph.org/0.3.0/index.html[Version 0.3.0]
Copy link
Member

Choose a reason for hiding this comment

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

0.3.0 hasn't been released yet... also, will the docs versions only list past versions, or all future versions as well? How will we keep these up-to-date, given that we don't modify historical build artifacts (including docs)?

Should we have a separate page on docs.janusgraph.org that is not part of the docs build that lists all possible versions, and have all the docs link there, so that there's no need to rebuild prior versions and they stay intact?

Copy link
Member Author

@chupman chupman May 30, 2018

Choose a reason for hiding this comment

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

@mbrukman For 0.3.0 I figured this would get merged when we did the release, which I'm led to believe should be in the near future(hopefully).

I have a PR in the docs project JanusGraph/docs.janusgraph.org#27 where I checked out all the release tags and replaced the Other Versions link to the release page with a link to the versions page. That PR should get merged before this one.

I'm proposing we only have the dropdown on the most current supported versions and all the finalized releases will just have a link to doc-versions.html. When new releases are added the only thing that would need to be updated is the static doc-versions.html file in each release folder. Since the file will be identical across the releases it represents a trivial update that could easily be scripted in CI.

Copy link
Member

Choose a reason for hiding this comment

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

This means we have to update doc-versions.html in every doc page, of every version; however, each page is built differently (i.e., the CSS/XML transforms are custom to every version), so if we change the CSS for some version (say, 0.4.0) we can't reuse that HTML verbatim back in 0.1.0 because then it will look wrong.

The correct thing to do in that case would be to run N new docs builds, one in each documentation directory, to ensure that it picks up the correct CSS/XML relevant for that version.

I would like to propose that we don't give ourselves this much work, and do the following:

  • "Other Doc Versions" always links to a static link: /latest/doc-versions.html

  • we keep updating the doc-versions.adoc file to add all the new versions

  • each doc build will only include prior versions, but that's fine: when the link is clicked via the top, users will find the complete set of versions, but in the doc they will only see earlier versions

  • thus, in the body of doc-versions.adoc it should also say something like:

    This page only lists versions released prior to ${version}; see complete list of docs versions for later versions."

Thoughts?

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 like the idea of just linking to /latest/doc-versions.html I just did a diff between versions and they are less congruent than I was expecting. I should be able to update this PR as well as JanusGraph/docs.janusgraph.org#27 tomorrow morning.

Thanks for the feedback.

Copy link
Member Author

Choose a reason for hiding this comment

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

requested changes have been made. I'll setup a github pages preview once JanusGraph/docs.janusgraph.org#27 merges

Copy link
Member

@pluradj pluradj left a comment

Choose a reason for hiding this comment

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

a couple typos, but LGTM

* http://docs.janusgraph.org/0.1.1/index.html[Version 0.1.1]
* http://docs.janusgraph.org/0.1.0/index.html[Version 0.1.0]

This page only lists versions ${version}; and prior. Please visit the latest version to https://docs.janusgraph.org/latest/doc-versions.html[see a complete list of docs versions]."
Copy link
Member

Choose a reason for hiding this comment

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

extraneous semicolon and quotation marks

Copy link
Member Author

Choose a reason for hiding this comment

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

semicolon and quotation mark removed

@chupman chupman force-pushed the version-dropdown branch from 5115eb5 to 7f419bf Compare June 4, 2018 21:00
@chupman chupman force-pushed the version-dropdown branch from 7f419bf to 7b61549 Compare June 5, 2018 21:12
@chupman
Copy link
Member Author

chupman commented Jun 5, 2018

@mbrukman Thinking more about ease of maintenance I added a link to latest as well since that link never changes. I think all the requested changes have been made. I humbly request the bestowment of a green check mark.

@janusgraph-bot janusgraph-bot added the cla: yes This PR is compliant with the CLA label Jun 11, 2018
@pluradj
Copy link
Member

pluradj commented Jun 11, 2018

@mbrukman will you be able to review/approve this PR this week? thanks.

@sjudeng
Copy link
Contributor

sjudeng commented Jul 6, 2018

@mbrukman Can you add a note here if you'd like more time to review the updates or if it's okay to move forward with merge?

@sjudeng sjudeng dismissed mbrukman’s stale review July 7, 2018 21:22

All requested updates appear to have been addressed

@pluradj pluradj modified the milestones: Release v0.2.1, Release v0.2.2 Jul 9, 2018
@janusgraph-bot janusgraph-bot added the cla: yes This PR is compliant with the CLA label Jul 16, 2018
Signed-off-by: Chris Hupman <chupman@us.ibm.com>
* https://docs.janusgraph.org/0.1.1/index.html[Version 0.1.1]
* https://docs.janusgraph.org/0.1.0/index.html[Version 0.1.0]

This page only lists versions 0.3.0 and prior. Please visit the latest version to https://docs.janusgraph.org/latest/doc-versions.html[see a complete list of docs versions].
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure here. Possibly I just don't know some grammar rules, but as for me it should be vice verse. I.e. something like:
Please visit the complete list of doc versions to see the latest version.
Maybe I'm wrong.

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 is a version the doc listings page. The issue is we don't plan to update the old ones so we wanted to provide a link to the latest version,https://docs.janusgraph.org/latest/doc-versions.html, which will always have a complete listing.

I'll be in the office tomorrow and I'll take a look at rewriting this sentence to make it easier to read.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, my bad. I thought the link is pointing to all versions documentations. My mistake. Now I understand it.

Copy link
Member

@porunov porunov left a comment

Choose a reason for hiding this comment

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

LGTM

@chupman chupman merged commit 43f03e0 into JanusGraph:0.2 Aug 20, 2018
bwatson-rti-org pushed a commit to bwatson-rti-org/janusgraph that referenced this pull request Mar 9, 2019
micpod pushed a commit to micpod/janusgraph that referenced this pull request Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This PR is compliant with the CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants