ys2843 edited a comment on pull request #18288: URL: https://github.com/apache/incubator-mxnet/pull/18288#issuecomment-628990327
> Looks good. > > * Should we use offsite js files or host locally? > * Should the API key be in a js file are can it be in a config file so we keep this stuff in someplace central? > * Wonder if we could have the list of versions also in a config file and not in .js, so it is more obvious and easier to maintain... then other parts of the site like the install selector or other things that need the list of versions can share? > > Just some thoughts... otherwise this looks good to me. > Good work! Should we use offsite js files or host locally? + Based on the research, the CDN works pretty well in China. I will keep an eye on it, if there is problem loading this file in any country, I will switch to host the file locally. Should the API key be in a js file are can it be in a config file so we keep this stuff in someplace central? + Didn't move API key to Jekyll at last, because Jekyll does not work with files such as `assets/*.js` very well, the way to get variable from Jekyll to JS is not straight forward ( use `_include`), please correct me if I am wrong. And after this version is archived, it is also easier to maintain if the key is kept in 1 JS file, but not spread to every page by Jekyll include <script>. Wonder if we could have the list of versions also in a config file and not in .js, so it is more obvious and easier to maintain... then other parts of the site like the install selector or other things that need the list of versions can share? + Great idea, I will set these common variables as Jekyll global var and generate the HTML when compiling Jekyll ---------------------------------------------------------------- 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: us...@infra.apache.org
