Skip to content
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

feat: Included docsify version in url #107

Merged
merged 2 commits into from Jun 18, 2020

Conversation

aemmadi
Copy link
Contributor

@aemmadi aemmadi commented Jun 16, 2020

Fixes #58

init now creates index.html with major version in URL.

<script src="//cdn.jsdelivr.net/npm/docsify@4/lib/docsify.min.js"></script>

Updated docsify docs accordingly #1225

Copy link
Contributor

@jthegedus jthegedus left a comment

Choose a reason for hiding this comment

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

LGTM

lib/template/index.html Outdated Show resolved Hide resolved
@jhildenbiddle jhildenbiddle self-requested a review June 17, 2020 17:08
Copy link
Member

@jhildenbiddle jhildenbiddle left a comment

Choose a reason for hiding this comment

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

Using only the major version number is the correct implementation (as @aemmadi has done). This will load the latest v4 minor and point releases, but not the next major release (v5).

Two change requests:

  1. Remove the file path and name from the URL:

    <script src="//cdn.jsdelivr.net/npm/docsify@4"></script>

    JSDelivr will automatically deliver the file specified in package.json using the jsdelivr, browser or main property and minify it if necessary. Docsify specifies "main": "lib/docsify.js" in its package.json file, so specifying only //cdn.jsdelivr.net/npm/docsify@4 will result in the minified version of lib/docsify.js being delivered. The reason to do this is because it gives maintainers the ability to change the file location of the main file without requiring an update to the CDN URL. If interested, you can read more about it here: https://www.jsdelivr.com/features#publishing-packages

  2. Add a docsify version to the stylesheet as well

    <link rel="stylesheet" href="//cdn.jsdelivr.net/npm/docsify/lib/themes/vue.css">

    Should be:

    <link rel="stylesheet" href="//cdn.jsdelivr.net/npm/docsify@4/lib/themes/vue.css">

    Note here that, unlike above, the full URL (path and filename) are required. This is because only one default file can be specified in package.json.

These same changes should be made in docsifyjs/docsify#1225.

@anikethsaha
Copy link
Member

his will load the latest v4 minor and point releases, but not the next major release (v5).

Great, this is what I was looking for.

👍 for this

@aemmadi
Copy link
Contributor Author

aemmadi commented Jun 18, 2020

@anikethsaha @jhildenbiddle Pushed the requested changes 😄

@jhildenbiddle jhildenbiddle self-requested a review June 18, 2020 17:34
Copy link
Member

@jhildenbiddle jhildenbiddle left a comment

Choose a reason for hiding this comment

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

Looks good!

@anikethsaha @trusktr -- You guys want to handle the merge? Not sure if one of you own merges and/or releases. Thx!

@anikethsaha anikethsaha merged commit d92d030 into docsifyjs:master Jun 18, 2020
@anikethsaha
Copy link
Member

@jhildenbiddle done 👍

@aemmadi thanks for your contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify major version number in URLs
4 participants