-
Notifications
You must be signed in to change notification settings - Fork 5.9k
reading source code with woboq #1707
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
paddle/scripts/docker/README.md
Outdated
- Build PaddlePaddle source code to be static files | ||
|
||
```bash | ||
docker run -v $PWD:/paddle -v $HOME/nginx_data:/usr/share/nginx/html/data -v $HOME/nginx_data/paddle:/usr/share/nginx/html/paddle -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TEST=OFF" -e "RUN_TEST=OFF" -e "BUILD_WOBOQ=ON" paddle:dev |
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.
感觉应该让用户知道需要创建文件夹$HOME/nginx_data
,以及这个指令会运行woboq将网页版代码生成到$HOME/nginx_data
里面。
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.
用户应该不需要手动创建$HOME/nginx_data,docker run
会自动在本地创建这个目录,但确实应该指出来会创建这个目录。
Done.
paddle/scripts/docker/README.md
Outdated
``` | ||
|
||
### Reading source code with woboq codebrowser | ||
If you are interesting of PaddlePaddle source code, [Woboq codebrowser](https://github.com/woboq/woboq_codebrowser) is a suitable tool. |
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.
For developers who are interested in the C++ source code, please use -e "WOBOQ=ON"
to enable building C++ source code into HTML pages using Woboq codebrowser
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.
Done.
paddle/scripts/docker/README.md
Outdated
- Build PaddlePaddle source code to be static files | ||
|
||
```bash | ||
docker run -v $PWD:/paddle -v $HOME/nginx_data:/usr/share/nginx/html/data -v $HOME/nginx_data/paddle:/usr/share/nginx/html/paddle -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TEST=OFF" -e "RUN_TEST=OFF" -e "BUILD_WOBOQ=ON" paddle:dev |
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.
这个例子里,需要提及 WITH_TEST
是不是因为这个决定过了unit test的源码是否被转换成HTML?我看了一下,woboq codebrowser是根据cmake的输出来知道如何build源码和转换源码的。
但是 RUN_TEST
是不是不必在这个例子里提及?
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.
赞细致,Done.
paddle/scripts/docker/README.md
Outdated
- Build PaddlePaddle source code to be static files | ||
|
||
```bash | ||
docker run -v $PWD:/paddle -v $HOME/nginx_data:/usr/share/nginx/html/data -v $HOME/nginx_data/paddle:/usr/share/nginx/html/paddle -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TEST=OFF" -e "RUN_TEST=OFF" -e "BUILD_WOBOQ=ON" paddle:dev |
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.
BUILD_WOBOQ => WOBOQ ?
paddle/scripts/docker/README.md
Outdated
- Build PaddlePaddle source code to be static files | ||
|
||
```bash | ||
docker run -v $PWD:/paddle -v $HOME/nginx_data:/usr/share/nginx/html/data -v $HOME/nginx_data/paddle:/usr/share/nginx/html/paddle -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TEST=OFF" -e "RUN_TEST=OFF" -e "BUILD_WOBOQ=ON" paddle:dev |
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.
这里为什么需要制定 /usr/share/nginx/html/data 和 同一个目录下的 paddle 两个子目录呢?是不是弄成 /woboq 就行了?
这里为什么要放在 /usr/share/nginx/html 里呢?这个是nginx的目录。但是development image里应该不需要有nginx?nginx如果需要应该放在doc image里?
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.
/usr/share/nginx/html/data
保存了一些js代码,paddle
目录保存的是html代码,但确实在这个Image里确实只创建一个根目录的volume就好了,另外/woboq
用来存在woboq源码了,所以目录改为/woboq_out
, :)
Done.
paddle/scripts/docker/README.md
Outdated
### Reading source code with woboq codebrowser | ||
For developers who are interested in the C++ source code, please use -e "WOBOQ=ON" to enable building C++ source code into HTML pages using [Woboq codebrowser](https://github.com/woboq/woboq_codebrowser). | ||
|
||
- The following command will generate woboq HTML pages in a docker volume directory, `$HOME/nginx` directory will be created on your local disk when the build finishes. |
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.
这个目录和nginx其实没关系,名字也不应该是nginx。可以叫 woboq_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.
$HOME/nginx
=> $HOME/woboq_out
Done.
paddle/scripts/docker/README.md
Outdated
### Reading source code with woboq codebrowser | ||
For developers who are interested in the C++ source code, please use -e "WOBOQ=ON" to enable building C++ source code into HTML pages using [Woboq codebrowser](https://github.com/woboq/woboq_codebrowser). | ||
|
||
- The following command will generate woboq HTML pages in a docker volume directory, `$HOME/nginx` directory will be created on your local disk when the build finishes. |
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 following command builds PaddlePaddle, generates HTML pages from C++ source code, and writes HTML pages into
$HOME/woboq_out
on the host:
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.
Done.
paddle/scripts/docker/README.md
Outdated
docker run -v $PWD:/paddle -v $HOME/nginx:/woboq_out -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TEST=ON" -e "WOBOQ=ON" paddle:dev | ||
``` | ||
|
||
- Open the generated static files in a browser, or upload these files to your web server. You can run nginx server as the following command, and then hit "http://<hostip>:8080/paddle" in browser. |
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 can open the generated HTML files in your Web browser. Or, if you want to run a Nginx container to serve them for a wider audience, you can run:
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.
Done.
Dockerfile
Outdated
|
||
# ENV variables | ||
ARG BUILD_WOBOQ | ||
ARG WOBOQ |
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.
这一行和16行貌似都不需要。因为
docker run -v $PWD:/paddle -v $HOME/woboq_out:/woboq_out -e "WITH_GPU=OFF" -e "WITH_AVX=ON" -e "WITH_TEST=ON" -e "WOBOQ=ON" paddle:dev
好像并没有用到这个Dockerfile(这个Dockerfile是build paddle:dev的时候用到的。)
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.
多谢 @helinwang , 第10行看起来确实不需要,不过对于第16行,因为docker run
的时候会执行https://github.com/Yancey1989/Paddle/blob/woboq_view/paddle/scripts/docker/build.sh#L50, 感觉这个环境变量写在Dockerfile里并赋初始值更符合用户习惯?
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.
明白了。写在16行的ENV WOBOQ确实更能够表达意图,更符合用户习惯。是我疏忽了,谢谢提醒!👍
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.
Done.
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
With only a minor comment about English grammar.
paddle/scripts/docker/README.md
Outdated
``` | ||
|
||
### Reading source code with woboq codebrowser | ||
For developers who are interested in the C++ source code, please use -e "WOBOQ=ON" to enable building C++ source code into HTML pages using [Woboq codebrowser](https://github.com/woboq/woboq_codebrowser). |
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.
to enable building
==> to enable the building of
…addlePaddle#1707) * update solov2 some detail * fix check paddle version
No description provided.