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

Adding DGRAPH_ENDPOINT env var for docs site #3764

Merged
merged 6 commits into from Aug 9, 2019

Conversation

@prashant-shahi
Copy link
Member

commented Aug 6, 2019

  • Adding DGRAPH_ENDPOINT environment variable for docs site.
  • Updating README file.

This change is Reviewable

@prashant-shahi prashant-shahi requested a review from danielmai Aug 6, 2019

@prashant-shahi prashant-shahi requested a review from MichaelJCompton as a code owner Aug 6, 2019

@pullrequest
Copy link

left a comment

A review job has been created and sent to the PullRequest network.


@prashant-shahi you can click here to see the review status or cancel the code review job.

@danielmai

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019


wiki/scripts/local.sh, line 26 at r1 (raw file):

  VERSIONS=${VERSION_STRING} \
  CURRENT_BRANCH="master" \
  pushd $GOPATH/src/github.com/dgraph-io/dgraph/wiki > /dev/null

Instead of adding that GOPATH note in the README, can we change local.sh to use the relative wiki path instead of the one in GOPATH?

@prashant-shahi
Copy link
Member Author

left a comment

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @danielmai and @MichaelJCompton)


wiki/scripts/local.sh, line 26 at r1 (raw file):

Previously, danielmai (Daniel Mai) wrote…

Instead of adding that GOPATH note in the README, can we change local.sh to use the relative wiki path instead of the one in GOPATH?

Done.

@danielmai
Copy link
Member

left a comment

Left some comments to address. Otherwise, LGTM.

Reviewed 2 of 3 files at r1, 1 of 2 files at r2.
Reviewable status: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @danielmai, @MichaelJCompton, and @prashant-shahi)


wiki/scripts/build.sh, line 73 at r2 (raw file):

	export CURRENT_VERSION=${2}
	export VERSIONS=${VERSION_STRING}
	export DGRAPH_ENDPOINT="https://play.dgraph.io/query?latency=true"

Instead of requiring the variable to be changed in the script, you can pick up an already set value from the shell environment or use the default value is the environment variable hasn't been set. The syntax would look like this:

export DGRAPH_ENDPOINT=${DGRAPH_ENDPOINT:-"https://play.dgraph.io/query?latency=true"}

then you should be able to see to set the Dgraph endpoint with either of these commands:

$ export DGRAPH_ENDPOINT="http://localhost:8080/query?latency=true"
$ ./local.sh

or

$ DGRAPH_ENDPOINT="http://localhost:8080/query?latency=true" ./local.sh

Running ./local.sh without an existing DGRAPH_ENDPOINT from the environment would use Play by default.


wiki/scripts/local.sh, line 27 at r2 (raw file):

  CURRENT_BRANCH="master" \

  pushd $PWD > /dev/null

cd'ing to $PWD is a no-op and doesn't actually change directories to anywhere specific since PWD is the current directory already.

To go to the relative path wiki directory you can do pushd $(dirname "$0")/.. . dirname $0 gives you the scripts directory and works even when running the script from any directory: ./wiki/scripts/local,sh, ./scripts/local,sh, or ,/local.sh all work.

@danielmai
Copy link
Member

left a comment

:lgtm:

Reviewable status: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @danielmai, @MichaelJCompton, and @prashant-shahi)

shell env and wiki folder path changes
- Pick DGRAPH_ENDPOINT from shell environment if it exists
- Replaced $PWD with better solution
@prashant-shahi
Copy link
Member Author

left a comment

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @danielmai and @MichaelJCompton)


wiki/scripts/build.sh, line 73 at r2 (raw file):

Previously, danielmai (Daniel Mai) wrote…

Instead of requiring the variable to be changed in the script, you can pick up an already set value from the shell environment or use the default value is the environment variable hasn't been set. The syntax would look like this:

export DGRAPH_ENDPOINT=${DGRAPH_ENDPOINT:-"https://play.dgraph.io/query?latency=true"}

then you should be able to see to set the Dgraph endpoint with either of these commands:

$ export DGRAPH_ENDPOINT="http://localhost:8080/query?latency=true"
$ ./local.sh

or

$ DGRAPH_ENDPOINT="http://localhost:8080/query?latency=true" ./local.sh

Running ./local.sh without an existing DGRAPH_ENDPOINT from the environment would use Play by default.

Done.


wiki/scripts/local.sh, line 27 at r2 (raw file):

Previously, danielmai (Daniel Mai) wrote…

cd'ing to $PWD is a no-op and doesn't actually change directories to anywhere specific since PWD is the current directory already.

To go to the relative path wiki directory you can do pushd $(dirname "$0")/.. . dirname $0 gives you the scripts directory and works even when running the script from any directory: ./wiki/scripts/local,sh, ./scripts/local,sh, or ,/local.sh all work.

Sure. will do these changes.

I had used $PWD since the instructions in README file specifically said to use the script from wiki folder.
4. Run ./scripts/local.sh from within the wiki folder

@prashant-shahi
Copy link
Member Author

left a comment

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @danielmai and @MichaelJCompton)


wiki/scripts/local.sh, line 27 at r2 (raw file):

Previously, prashant-shahi (Prashant Shahi) wrote…

Sure. will do these changes.

I had used $PWD since the instructions in README file specifically said to use the script from wiki folder.
4. Run ./scripts/local.sh from within the wiki folder

Done.

Also, added similar changes for ./scripts/build.sh file.

@danielmai

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019


wiki/README.md, line 27 at r3 (raw file):
Now that the DGRAPH_ENDPOINT variable is settable via the shell, step 3 and 4 can be combined and phrased something like this:

  1. Run ./scripts/local.sh from within the wiki folder and go to http://localhost:1313 to see the docs.
    (Optional) To run the runnable queries within docs with a different Dgraph instance, set the DGRAPH_ENDPOINT environment variable before starting the local web server:
DGRAPH_ENDPOINT="http://localhost:8080/query?latency=true" ./local.sh
@prashant-shahi
Copy link
Member Author

left a comment

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @danielmai and @MichaelJCompton)


wiki/README.md, line 27 at r3 (raw file):

Previously, danielmai (Daniel Mai) wrote…

Now that the DGRAPH_ENDPOINT variable is settable via the shell, step 3 and 4 can be combined and phrased something like this:

  1. Run ./scripts/local.sh from within the wiki folder and go to http://localhost:1313 to see the docs.
    (Optional) To run the runnable queries within docs with a different Dgraph instance, set the DGRAPH_ENDPOINT environment variable before starting the local web server:
DGRAPH_ENDPOINT="http://localhost:8080/query?latency=true" ./local.sh

Done.

@danielmai
Copy link
Member

left a comment

Reviewed 1 of 2 files at r2, 2 of 2 files at r3, 1 of 1 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MichaelJCompton)

@pullrequest
Copy link

left a comment

Minor grammar suggestion for clarity.


Reviewed with ❤️ by PullRequest

wiki/README.md Outdated Show resolved Hide resolved
wiki/README.md Outdated Show resolved Hide resolved

@prashant-shahi prashant-shahi merged commit 2a12255 into master Aug 9, 2019

4 of 5 checks passed

code-review/reviewable 1 file, 2 discussions left (danielmai, MichaelJCompton, prashant-shahi)
Details
Blockade (dgraph) TeamCity build finished
Details
CI (dgraph) TeamCity build finished
Details
GolangCI No issues found!
Details
license/cla Contributor License Agreement is signed.
Details

@prashant-shahi prashant-shahi deleted the prashant/add-endpoint-env branch Aug 9, 2019

danielmai added a commit that referenced this pull request Aug 9, 2019

Adding DGRAPH_ENDPOINT env var for docs site (#3764)
* Adding DGRAPH_ENDPOINT env variable

* Using relative wiki path

* Pick DGRAPH_ENDPOINT from shell environment if it exists

* Replaced $PWD with a better solution

* Updating README file

(cherry picked from commit 2a12255)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.