Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

[SKIP CI] Updating the script that copies changes to gh-pages from vmware/master #1863

Merged

Conversation

ashahi1
Copy link
Contributor

@ashahi1 ashahi1 commented Sep 5, 2017

Following changes have been made to the script:

  • Renamed the script to updateGHpages.sh
  • Delete the md files from jekyll-docs folder before copying the changes from vmware/master
  • Fail the script if any command fails
  • Added more comments for running the script

@shuklanirdesh82
Copy link
Contributor

Renamed the script to updateGHpages.sh

How about renaming to publish_document.sh/update_gh_pages.sh?

Copy link
Contributor

@shuklanirdesh82 shuklanirdesh82 left a comment

Choose a reason for hiding this comment

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

Please address some minor comments.

Following script copies the markdown files from vmware:master to gh-pages
Local copy of vDVS project is needed before running this script (git clone https://github.com/vmware/docker-volume-vsphere.git)
Script has to be run from the docker-volume-vsphere/
e.g. ./misc/scripts/updateGHpages.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: ./misc/scripts/updateGHpages.sh => ./misc/scripts/<script_name> .. it will be outdated soon when script name got rename.

# e.g. ./misc/scripts/updateGHpages.sh

set -e
: '
Copy link
Contributor

Choose a reason for hiding this comment

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

:??

Script has to be run from the docker-volume-vsphere/
e.g. ./misc/scripts/updateGHpages.sh

Before executing this script, its important to have two git remotes : origin and vmware
Copy link
Contributor

Choose a reason for hiding this comment

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

would suggest to move this pre-requisite before line#6

@@ -1,10 +1,21 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Renamed the script to updateGHpages.sh

How about renaming to publish_document.sh/update_gh_pages.sh?

Copy link
Contributor

@shuklanirdesh82 shuklanirdesh82 left a comment

Choose a reason for hiding this comment

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

LGTM!

Please address 2 minor comments before merging.

@@ -50,18 +73,22 @@ rm -rf $BACKUP_DIR

echo "Performing steps to generate customer facing document"
cd jekyll-docs/

# generating the html pages
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: generating => Generating .. the same as line#61

docker run --rm --volume=$(pwd):/srv/jekyll -it jekyll/jekyll:stable jekyll build
rm -rvf ../documentation
mv _site ../documentation

#updating documentation/index.html
# updating documentation/index.html
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

@ashahi1 ashahi1 merged commit 4770a61 into vmware-archive:master Sep 6, 2017
@ashahi1
Copy link
Contributor Author

ashahi1 commented Sep 6, 2017

Addressed all the review comments. Merging the changes.

@ashahi1 ashahi1 deleted the minorFixToSiteGenaratorScript.ashahi1 branch September 7, 2017 23:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants