-
Notifications
You must be signed in to change notification settings - Fork 383
[WIP] Pull LoopBack 4 doc updates from loopback-next #643
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
Conversation
5e74638 to
6f23a25
Compare
|
Your |
update-lb4-docs.sh
Outdated
| | tar --strip-components 1 -C $tmp -xzf- '*.md' | ||
| cp -R $tmp/packages $target/readmes/$repo | ||
| #rename relative link extensions from md to html | ||
| sed -E -i 's/\(([^(]+)\.md/(\1.html)/g' $tmp/docs/*.md |
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.
You have:
s/\(([^(]+)\.md/(\1.html)/g
It should be this:
s/\(([^(]+)\.md\)/(\1.html)/g
You need to match the closing ) literal otherwise it doesn't get replaced by the ) literal in the replacement and you end up with double )) on the output - and also incorrectly matching:
(index.md other stuff) -> (index.html) other stuff)
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.
Good catch. Actually I wanted to include the cases where .md is followed by # and left out the literal match for ). I will remove it from the replacement as well.
5b86ea9 to
2287b46
Compare
bschrammIBM
left a comment
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.
lgtm
4c54f77 to
9931ef0
Compare
9931ef0 to
482420b
Compare
shimks
left a comment
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.
LGTM, but one comment
update-lb4-docs.sh
Outdated
| curl -L https://github.com/$ORG/$REPO/archive/$COMMIT.tar.gz | tar --strip-components 1 -C "$DOCS" --wildcards \ | ||
| -xzf- "$ARCHIVE_NAME/docs/*.md" "$ARCHIVE_NAME/packages/*.md" "$ARCHIVE_NAME/docs/*.png" | ||
|
|
||
| # cp -R $DOCS/packages $DOCS/readmes/$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.
Was this meant to be 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.
+1
update-lb4-docs.sh
Outdated
| ORG=strongloop | ||
| REPO=loopback-next | ||
| DOCS=./pages/en/lb4 | ||
| COMMIT=893d68547766fc41d8bca82425645416e357d052 |
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.
Is this meant to be hard-coded?
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.
Good catch; it's supposed to be master.
update-lb4-docs.sh
Outdated
| curl -L https://github.com/$ORG/$REPO/archive/$COMMIT.tar.gz | tar --strip-components 1 -C "$DOCS" --wildcards \ | ||
| -xzf- "$ARCHIVE_NAME/docs/*.md" "$ARCHIVE_NAME/packages/*.md" "$ARCHIVE_NAME/docs/*.png" | ||
|
|
||
| # cp -R $DOCS/packages $DOCS/readmes/$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.
+1
6fb79d5 to
eee1484
Compare
| } | ||
| stage('download updates') { | ||
| sh './update-readmes.sh' | ||
| sh './update-v4-readmes.sh' |
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 probably should keep sh './update-v4-readmes.sh' here as it's used to pull in README files. Even after we move docs to the monorepo, we might continue to pull in README files.
update-lb4-docs.sh
Outdated
| mkdir -p $DOCS | ||
| # pull down all the markdown files (reserving dir structure) | ||
| # given the GitHub ORG, REPO, and COMMIT | ||
| curl -L https://github.com/$ORG/$REPO/archive/$COMMIT.tar.gz | tar --strip-components 1 -C "$DOCS" --wildcards \ |
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 some point, it's probably better to switch to npm install
24c7c8c to
5d44653
Compare
f7de9ec to
678a0d2
Compare
678a0d2 to
73d54a1
Compare
update-lb4-docs.sh
Outdated
| # -xzvf- "$ARCHIVE_NAME/docs/*.md" "$ARCHIVE_NAME/packages/*.md" | ||
| # "$ARCHIVE_NAME/docs/*.png" | ||
| npm i @loopback/docs --prefix $DOCS | ||
| mv $DOCS/node_modules/@loopback/docs $DOCS/monorepo-docs |
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.
Is /monorepo-docs needed? Or can we just move the docs fo $DOCS?
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 site/* should be copied into pages/en/lb4/.
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 was just using it as a placeholder, before we implement changes that override the current doc contents, but yeah it should be site/*.
|
I just published |
update-lb4-docs.sh
Outdated
| #curl -L https://github.com/$ORG/$REPO/archive/$BRANCH.tar.gz | tar --strip-components 1 -C "$DOCS" --wildcards \ | ||
| # -xzvf- "$ARCHIVE_NAME/docs/*.md" "$ARCHIVE_NAME/packages/*.md" | ||
| # "$ARCHIVE_NAME/docs/*.png" | ||
| npm i @loopback/docs --prefix $DOCS |
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 can be further simplified if we add @loopback/docs as a dependency in package.json of loopback.io. This way, npm install at the root level will pull in node_modules/@loopback/docs and we can add a post-install script to copy them to the right place.
_
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 talked to @rmg and if we aim to do that, then we should probably introduce Travis to this repo since the Jenkins pipeline is
running directly on the Jenkins master and that server doesn't have any external executors
and doesn't have node/npm installed. It would be a hassle to install them there. Also,
all GitHub Pages does is run Jekyll and upload the generated
_sitedirectory (or whatever it is..) to S3
update-lb4-docs.js
Outdated
| const destImages = path.resolve(__dirname, 'images', 'lb4'); | ||
|
|
||
| function copyDocs(src, dest) { | ||
| try { |
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.
IIRC, we should use two spaces for indentation.
19493fd to
aae94cf
Compare
_layouts/page.html
Outdated
| {% if site.lb4_editme_path and page.layout != 'readme' and page.sidebar == 'lb4_sidebar' %} | ||
| {% assign edit_url = page.url | remove: "doc/en/lb4" | remove: ".html" | append: ".md" | prepend: site.lb4_editme_path %} | ||
| <a target="_blank" href="https://github.com/{{edit_url}}" class="btn btn-default githubEditButton" role="button"><i class="fa fa-github fa-lg"></i> Edit this page</a> | ||
| {% endif %} |
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.
wrong indentation?
Jenkinsfile
Outdated
|
|
||
| env.CHANGE_BRANCH = env.CHANGE_BRANCH ?: env.BRANCH_NAME | ||
|
|
||
|
|
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.
Looks like a random whitespace change was accidentally left in.
.travis.yml
Outdated
| - npm test | ||
| email: slnode@ca.ibm.com | ||
| name: StrongLoop Bot | ||
|
No newline at end of file |
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.
Think there's some extra empty space here.
|
Travis has been configured |
37ab270 to
a563306
Compare
ac9f169 to
2f5382f
Compare
|
Unfortunately, we can't use custom commit messages for the commits made by Travis on successful builds, but the feature might come soon https://github.com/travis-ci/travis-ci/issues/9287. |
5f23fcb to
5daa9ae
Compare
Add a script to copy over loopback-next docs using @loopback/docs. Add Travis config to pick up the changes and deploy to GitHub Pages upon a successful build. Run README update scripts from Travis as well and remove Jenkinsfile which did the same thing with Jenkins.
e8f7b01 to
09d4ef6
Compare
Connect to loopbackio/loopback-next#843. This script copies over the doc files for LoopBack 4 content into this repo, and reverts some linking changes made in
loopback-nextin order to render it properly when Jekyll is run. Mad credits to @rmg for essentially writing the script! The script is intended to demonstrate how to achieve the proposal in the spike, so it would probably need to be modified if we are to use this approach.EDIT The script now utilizes
@loopback/docsto pull in the latest changes fromloopback-nextfor LoopBack 4 docs andjekyll-relative-linksplugin to handle.mdlinks (with one exception). It also provides a Travis config to deploy to GitHub Pages whenever there are new changes.