You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2021/04/02 13:13:17 UTC
[GitHub] [tinkerpop] OyvindSabo opened a new pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
OyvindSabo opened a new pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412
https://issues.apache.org/jira/browse/TINKERPOP-2544
Updates the `bin/generate-home.sh` script to build the gremlint web page to an optimized production build which can be hosted by a static server and then copy it from `docs/gremlint/build` to `target/site/home/gremlint`.
If I understand the `bin/generate-home.sh` script correctly, it uses `rsync` to do the file copying if it is available. I've tested the script with and without `rsync`, and in both cases I've tested it with and without the page having previously been generated. I've only tested on a Linux machine.
I'll need some help to test if the `bin/publish-home.sh` script will simply just work, or if it needs modification to properly publish the built gremlint web page.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] OyvindSabo commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
OyvindSabo commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812726281
I made some updates to the docs. It's getting late here so I'll merge this sometime tomorrow.
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] OyvindSabo commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
OyvindSabo commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812579241
While running html files locally by opening them as files is very convenient, there are some limitations to what kinds of files can be opened this way. The reason the gremlint page can't be viewed if you open it as a file locally is that the built site consists of an HTML document which imports JavaScript files (as opposed to having it all inlined in the html document). When opened as a file locally those imports are considered to be cross-origin requests by the browser and blocked. It works if you serve the `index.html` file from a web server, since then all the files will be served from the same origin.
If you have npm > 5.2.0 you can test the gremlint page by running `npx serve target/site/home/gremlint`, and the home page, for instance by running `npx serve target/site/home`.
The error in the published page on https://tinkerpop.apache.org/gremlint/ seems to be different, though. After looking at it for a while I noticed that the static assets are imported from `https://tinkerpop.apache.org/static/` instead of `https://tinkerpop.apache.org/gremlint/static/`, where they are actually located. I'll update the imports to be relative to `index.html` so that the page works regardless of whether it's hosted at the server root or not, as described here https://create-react-app.dev/docs/deployment/#serving-the-same-build-from-different-paths
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812530880
my initial test of `generate-home.sh` on this branch produced:
```text
$ bin/generate-home.sh
~/git/apache/tinkerpop/docs/gremlint ~/git/apache/tinkerpop
bin/generate-home.sh: line 28: npm: command not found
~/git/apache/tinkerpop
rsync: change_dir "/home/smallette/git/apache/tinkerpop//docs/gremlint/build" failed: No such file or directory (2)
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1207) [sender=3.1.3]
~/git/apache/tinkerpop/docs/site ~/git/apache/tinkerpop
~/git/apache/tinkerpop
Home page site generated to /home/smallette/git/apache/tinkerpop/target/site/home
```
I guess i rely on the npm installed by maven to build gremlin-javascript. so i got npm installed and then ran again to get:
```text
$ bin/generate-home.sh
~/git/apache/tinkerpop/docs/gremlint ~/git/apache/tinkerpop
> gremlint.com@0.1.0 build
> react-scripts build
sh: 1: react-scripts: not found
~/git/apache/tinkerpop
rsync: change_dir "/home/smallette/git/apache/tinkerpop//docs/gremlint/build" failed: No such file or directory (2)
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1207) [sender=3.1.3]
~/git/apache/tinkerpop/docs/site ~/git/apache/tinkerpop
~/git/apache/tinkerpop
Home page site generated to /home/smallette/git/apache/tinkerpop/target/site/home
```
definitely don't see the `docs/gremlint/build` directory which is probably because of the react error?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette edited a comment on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
spmallette edited a comment on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812599182
republished the site - works!
https://tinkerpop.apache.org/gremlint/
VOTE +1 - pending docs changes of course. (i think you can just squash to a single commit and merge this - no need to await further review from folks as this is a small administrative change.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] OyvindSabo commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
OyvindSabo commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812600224
Awesome, I'll update the docs and merge later today then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812547104
this works nice for me now. ran `generate-home.sh` and got the site locally and then ran `publish-home.sh` and it went where it was supposed to go:
https://tinkerpop.apache.org/gremlint/
of course, i can't get the site to render there or locally. did i do something wrong?
> By the way, is it okay to rely on npm being available or should there be some local copy of Node.js and npm like in gremlin-javascript?
only PMC members can publish the home page, so given that only a few people will need to do this i think it's fine to assume we will have npm installed. I think that does mean though that the dev docs should be updated to reflect this change in requirements:
https://tinkerpop.apache.org/docs/current/dev/developer/#nodejs-environment
https://tinkerpop.apache.org/docs/current/dev/developer/#site
I would probably update the first reference above to mention the need for npm to be installed to run `generate-home.sh` and then add a link in the second reference to point to that section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] OyvindSabo merged pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
OyvindSabo merged pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812599182
republished the site - works!
https://tinkerpop.apache.org/gremlint/
VOTE +1 (i think you can just squash to a single commit and merge this - no need to await further review from folks as this is a small administrative change.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812580277
> If you have npm > 5.2.0 you can test the gremlint page by running npx serve target/site/home/gremlint, and the home page, for instance by running npx serve target/site/home
That might be some additional content to add to https://tinkerpop.apache.org/docs/current/dev/developer/#site
> The error in the published page on https://tinkerpop.apache.org/gremlint/ seems to be different, though.
ok - i will keep an eye out for your fix and try again when it's ready.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] OyvindSabo commented on pull request #1412: TINKERPOP-2544: Modify site publishing scripts to include gremlint
Posted by GitBox <gi...@apache.org>.
OyvindSabo commented on pull request #1412:
URL: https://github.com/apache/tinkerpop/pull/1412#issuecomment-812539563
I suppose I had previously run `npm install` and that's why it worked. I was able to reproduce the `react-scripts: not found` message by deleting the `docs/gremlint/node_modules` folder (which is the state we're in before running `npm install`). Adding `npm install` before `npm run build` (https://github.com/apache/tinkerpop/pull/1412/commits/679a6ce9f0ca26fddd9043366efb30c60efcd59c) did the trick for me.
By the way, is it okay to rely on npm being available or should there be some local copy of Node.js and npm like in `gremlin-javascript`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org