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

New docs versioning - repo proposal #1855

Merged
merged 32 commits into from
Sep 19, 2017
Merged

New docs versioning - repo proposal #1855

merged 32 commits into from
Sep 19, 2017

Conversation

igor-dv
Copy link
Member

@igor-dv igor-dv commented Sep 16, 2017

Issue: related to #1773

What I did

I've added a functionality to the docs-v2 build script that will commit a new exported docs version to a separate repo

Tasks TBD

  • Fix relative to root links
  • Fix dynamic chunks are not being downloaded for the version site.
  • Use/Create Storybook bot user to commit to the docs repo.
  • Create storybook docs repo
  • Add the docs repo to the CI
  • Add DOCS_REPO variable with access token to CI
  • Automate docs deployment in main storybook CI
  • Generate the versions.json for a versions drop-down
  • Fix edge case when the same exported version is generated with a different build hash (because maybe something was changed but version is still the same) : when copying to the main _next dir the old app version data should be deleted.

Example

https://storypuke-mlglazhotd.now.sh
https://storypuke-mlglazhotd.now.sh/3-0-0
https://storypuke-mlglazhotd.now.sh/3-0-1
https://storypuke-mlglazhotd.now.sh/3-2-5
https://storypuke-mlglazhotd.now.sh/3-2-9

How to test

Meanwhile:

  1. Create a repo
  2. Add a basic package.json - it's needed because the build script is looking for it to upgrade the version.
  3. Create authentication token from here : https://github.com/settings/tokens
  4. In docs-v2/package.json change the build script to this: "build": "cross-env DOCS_REPO=https://{your_user_here}:{token_from_step_3}@github.com/{repo} node scripts/build.js",
  5. Run the build script from the step 4
  6. Change the version in the docs-v2/package.json
  7. Run the build script from the step 4 again to see that newer version committed to the repo as well
  8. Optional step - host on cloud - look the now configuration here: https://github.com/igor-dv/storypuke-docs/blob/master/package.json

@codecov
Copy link

codecov bot commented Sep 17, 2017

Codecov Report

Merging #1855 into new-docs will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           new-docs    #1855   +/-   ##
=========================================
  Coverage     22.59%   22.59%           
=========================================
  Files           324      324           
  Lines          6245     6245           
  Branches        804      801    -3     
=========================================
  Hits           1411     1411           
+ Misses         4226     4224    -2     
- Partials        608      610    +2
Impacted Files Coverage Δ
app/react/src/server/utils.js 0% <0%> (-53.58%) ⬇️
lib/ui/src/modules/api/configs/init_api.js 40.47% <0%> (ø) ⬆️
addons/info/src/components/types/proptypes.js 16.66% <0%> (ø) ⬆️
addons/knobs/src/base.js 2.5% <0%> (ø) ⬆️
lib/ui/src/modules/ui/components/layout/usplit.js 38.7% <0%> (ø) ⬆️
lib/components/src/navigation/menu_link.js 0% <0%> (ø) ⬆️
...react-native/src/manager/components/PreviewHelp.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/types/Select.js 7.93% <0%> (ø) ⬆️
addons/knobs/src/KnobManager.js 32% <0%> (ø) ⬆️
lib/components/src/table/table.js 0% <0%> (ø) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18c784d...0f5010d. Read the comment docs.

@@ -35,30 +38,33 @@ const createFile = (stat, fileName, workingDir, baseDir) => {
name,
title,
isFile: true,
route: path.join(routeBase, routeFile),
route: path.posix.join(routeBase, routeFile),
Copy link
Member

Choose a reason for hiding this comment

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

Can you give some context on this?

Copy link
Member Author

@igor-dv igor-dv Sep 18, 2017

Choose a reason for hiding this comment

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

On windows all the routes were generated with the "\" instead of "/" and it breaks routing completely.

Copy link
Member

Choose a reason for hiding this comment

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

This works on windows, though? Interesting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it just enforce the path to use posix style thingies..

@@ -0,0 +1,22 @@
function getGitClone(docsRepo, outputDir) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice work on extracting out all these little tasks!

@codecov-io
Copy link

codecov-io commented Sep 18, 2017

Codecov Report

Merging #1855 into new-docs will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           new-docs    #1855   +/-   ##
=========================================
  Coverage     22.59%   22.59%           
=========================================
  Files           324      324           
  Lines          6245     6245           
  Branches        804      797    -7     
=========================================
  Hits           1411     1411           
- Misses         4226     4242   +16     
+ Partials        608      592   -16
Impacted Files Coverage Δ
app/react/src/server/utils.js 0% <0%> (-53.58%) ⬇️
addons/knobs/src/components/types/Color.js 8.1% <0%> (ø) ⬆️
app/react-native/src/preview/story_kind.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/components/menu_item.js 19.14% <0%> (ø) ⬆️
...modules/ui/components/stories_panel/text_filter.js 30.98% <0%> (ø) ⬆️
.../src/modules/ui/components/stories_panel/header.js 29.62% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/layout.js 12.5% <0%> (ø) ⬆️
addons/knobs/src/KnobStore.js 6.81% <0%> (ø) ⬆️
addons/info/src/components/types/InstanceOf.js 16.66% <0%> (ø) ⬆️
lib/ui/src/modules/shortcuts/actions/shortcuts.js 22.41% <0%> (ø) ⬆️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18c784d...2236896. Read the comment docs.


const versionRegex = [/^\/([0-9]+-[0-9]+-[0-9]+)\//, /^\/([0-9]+-[0-9]+-[0-9]+)$/];

function getVersionHref(path, href) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain the rationale for this?
I'm having a hard time understanding what this exactly is doing / is for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's say we are browsing in https://storybook.js.org/3-2-9/help. Every link here should be related to the /3-2-9/ subdir. But the problem is that every link starts with '/' so it's actually relative to the root. In this case clicking on this link will open the wrong page (or will shot a wrong window.location that after refresh will open a wrong page).
getVersionHref is looking for the /x-x-x/ pattern and then returns the url that is base on this version.
After that it's attached to the as parameter of the next\link that will just show the right url in the window.location

Copy link
Member

Choose a reason for hiding this comment

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

gotcha!

@@ -35,30 +38,33 @@ const createFile = (stat, fileName, workingDir, baseDir) => {
name,
title,
isFile: true,
route: path.join(routeBase, routeFile),
route: path.posix.join(routeBase, routeFile),
Copy link
Member

Choose a reason for hiding this comment

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

This works on windows, though? Interesting.

}));
};

const createDirectory = (items, localWorkingDir, baseDir) => ({
route: localWorkingDir.replace(baseDir, ''),
route: localWorkingDir.replace(baseDir, '').replace(/\\/, '/'),
Copy link
Member

Choose a reason for hiding this comment

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

comment that this is for windows compatibility, would be nice.

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

@igor-dv igor-dv merged commit a7dfa57 into new-docs Sep 19, 2017
@igor-dv igor-dv deleted the new-docs-versioning2 branch September 19, 2017 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants