You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/06/18 22:00:17 UTC
[GitHub] [incubator-mxnet] ys2843 opened a new pull request #18587: add version check on installation guide
ys2843 opened a new pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587
## Description ##
Fix #17244 . Grab the available versions from the version drop down, and add a check to ensure the target version is available before updating the installation guide.
## Checklist ##
### Essentials ###
Please feel free to remove inapplicable items for your PR.
- [ ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) created (except PRs with tiny changes)
- [ ] Changes are complete (i.e. I finished coding on this PR)
### Changes ###
- [ ] Add version check before updating the installation instruction
## Comments ##
- preview: pending
----------------------------------------------------------------
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] [incubator-mxnet] leezu commented on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648968898
> I am not sure how this bug was imported, will fix it in another PR.
I didn't see the bug locally previously on the public site due to the cache. Thank you @ys2843 for checking and going to fix it.
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646337882
@mxnet-label-bot add [website]
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 commented on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
ys2843 commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648563172
> > > 1.6 --> Linux
> > > Scala, Java, Clojure, R... and the rest don't display anything for instructions.
> >
> >
> > @aaronmarkham This PR doesn't touch the docs, these instructions are removed in #18487 . In Production, these docs are also inaccessible.
> > It checks the version with the version select dropdown, only redirects to 1.6 when the string in url is not valid. So as long as “master" is in the dropdown, it won’t redirects to v1.6
>
> @ys2843 on the public version https://mxnet.apache.org/get_started?platform=linux&language=r&processor=cpu&environ=build-from-source&, it displays "You will need to R v3.4.4+ and build MXNet from source. Please follow the instructions linked above." etc. But in your PR review, the sentence is removed.
This is a bug, but not from this PR. Public website also won't show this message by clicking through the widget, it only shows by visiting the url directly. Same as preview.
For example, start from https://mxnet.apache.org/get_started? and click through the installation widget (v1.6 => R => GPU), this message won't show.
And visit this link directly http://ec2-34-219-134-42.us-west-2.compute.amazonaws.com/get_started?platform=linux&language=r&processor=cpu&environ=build-from-source&. The message shows.
I am not sure how this bug was imported, will fix it in another PR.
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646336523
@leezu Preview link attached, it fixes the `get started` page version select. If the target version falls outside the ones in the version dropdown, it will be replaced by the newest version.
----------------------------------------------------------------
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] [incubator-mxnet] mxnet-bot commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646327063
Hey @ys2843 , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:
- To trigger all jobs: @mxnet-bot run ci [all]
- To trigger specific jobs: @mxnet-bot run ci [job1, job2]
***
**CI supported jobs**: [centos-cpu, unix-gpu, windows-gpu, miscellaneous, clang, website, windows-cpu, unix-cpu, sanity, centos-gpu, edge]
***
_Note_:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.
----------------------------------------------------------------
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] [incubator-mxnet] leezu commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646340918
> This bug fix only applies to the situation when user directly type "version=master" in the url, I am not sure if it can fix the problem you described. If that problem is important, please create an issue and let me know what's the expect behavior, thanks.
The problem I refer to is not a website issue but a lack of documentation issue.
----------------------------------------------------------------
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] [incubator-mxnet] mxnet-bot commented on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648940354
Jenkins CI successfully triggered : [centos-gpu, centos-cpu]
----------------------------------------------------------------
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] [incubator-mxnet] leezu commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646331553
Thank you @ys2843. Could you post a screenshot with a before & after comparison? Currently I'm not sure in which installation guide the version is not correctly checked. Thanks!
(In general, the problem is that the build from source guide currently exists only in a single version. We should split up the document into multiple versions for the 1.x releases without good cmake support and for the 1.x releases with good cmake support + 2.x release)
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648373376
> 1.6 --> Linux
> Scala, Java, Clojure, R... and the rest don't display anything for instructions.
This PR doesn't touch the docs, these instructions are removed in #18487
----------------------------------------------------------------
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] [incubator-mxnet] leezu edited a comment on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
leezu edited a comment on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648544783
> > 1.6 --> Linux
> > Scala, Java, Clojure, R... and the rest don't display anything for instructions.
>
> @aaronmarkham This PR doesn't touch the docs, these instructions are removed in #18487 . In Production, these docs are also inaccessible.
> It checks the version with the version select dropdown, only redirects to 1.6 when the string in url is not valid. So as long as “master" is in the dropdown, it won’t redirects to v1.6
@ys2843 on the public version https://mxnet.apache.org/get_started?platform=linux&language=r&processor=cpu&environ=build-from-source&, it displays "You will need to R v3.4.4+ and build MXNet from source. Please follow the instructions linked above." etc. But in your PR review, the sentence is removed.
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 edited a comment on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 edited a comment on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646336523
@leezu Preview link attached, it fixes the `get started` page version select. If the target version falls outside the ones in the version dropdown, it will be replaced by the newest version.
This bug fix only applies to the situation when user directly type "version=master" in the url, I am not sure if it can fix the problem you described. If that problem is important, please create an issue and let me know what's the expect behavior, thanks.
----------------------------------------------------------------
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] [incubator-mxnet] leezu edited a comment on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
leezu edited a comment on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646331553
Thank you @ys2843. Could you post a screenshot with a before & after comparison? Currently I'm not sure in which installation guide the version is not correctly checked. Thanks!
(In general, one problem is that the build from source guide currently exists only in a single version. We should split up the document into multiple versions for the 1.x releases without good cmake support and for the 1.x releases with good cmake support + 2.x release)
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 edited a comment on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 edited a comment on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646337882
@mxnet-label-bot update [website, pr-awaiting-review]
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 edited a comment on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
ys2843 edited a comment on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648940274
@mxnet-bot run ci [unix-cpu]
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 edited a comment on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 edited a comment on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648373376
> 1.6 --> Linux
> Scala, Java, Clojure, R... and the rest don't display anything for instructions.
@aaronmarkham This PR doesn't touch the docs, these instructions are removed in #18487 . In Production, these docs are also inaccessible.
It checks the version with the version select dropdown, only redirects to 1.6 when the string in url is not valid. So as long as “master" is in the dropdown, it won’t redirects to v1.6
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 commented on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
ys2843 commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648940274
@mxnet-bot run ci [centos-cpu, centos-gpu]
----------------------------------------------------------------
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] [incubator-mxnet] leezu commented on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648544783
> > 1.6 --> Linux
> > Scala, Java, Clojure, R... and the rest don't display anything for instructions.
>
> @aaronmarkham This PR doesn't touch the docs, these instructions are removed in #18487 . In Production, these docs are also inaccessible.
> It checks the version with the version select dropdown, only redirects to 1.6 when the string in url is not valid. So as long as “master" is in the dropdown, it won’t redirects to v1.6
@ys2843 on the master version, it displays "You will need to R v3.4.4+ and build MXNet from source. Please follow the instructions linked above." etc. But in your PR review, the sentence is removed.
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 edited a comment on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 edited a comment on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-646336523
@leezu Preview link attached, it fixes the `get started` page version select. If the target version falls outside the ones from the version dropdown, it will be replaced by the newest version.
This bug fix only applies to the situation when user directly type "version=master" in the url, I am not sure if it can fix the problem you described. If that problem is important, please create an issue and let me know what's the expect behavior, thanks.
----------------------------------------------------------------
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] [incubator-mxnet] mxnet-bot commented on pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-649130642
Jenkins CI successfully triggered : [unix-cpu]
----------------------------------------------------------------
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] [incubator-mxnet] ys2843 commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
ys2843 commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648486956
> > > 1.6 --> Linux
> > > Scala, Java, Clojure, R... and the rest don't display anything for instructions.
> >
> >
> > @aaronmarkham This PR doesn't touch the docs, these instructions are removed in #18487 . In Production, these docs are also inaccessible.
> > It checks the version with the version select dropdown, only redirects to 1.6 when the string in url is not valid. So as long as “master" is in the dropdown, it won’t redirects to v1.6
>
> As far as I'm concerned, that installation widget is now broken. If the purpose of the PR is prevent unsupported versions from showing up then it shouldn't have a customer click down a path of options to have no result.
>
> If the purpose is to "prevent master" then I guess you could update the title and restate the purpose of the PR... then we deal with how poor the CX is now for customers in another issue/pr.
This PR is to prevent any unsupported version (or random string) being entered from URL, which is a bug for the widget. I will restate the purpose and we can discuss what's the current expectation for installation widget with mxnet team.
----------------------------------------------------------------
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] [incubator-mxnet] aaronmarkham commented on pull request #18587: add version check on installation guide
Posted by GitBox <gi...@apache.org>.
aaronmarkham commented on pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587#issuecomment-648484040
> > 1.6 --> Linux
> > Scala, Java, Clojure, R... and the rest don't display anything for instructions.
>
> @aaronmarkham This PR doesn't touch the docs, these instructions are removed in #18487 . In Production, these docs are also inaccessible.
> It checks the version with the version select dropdown, only redirects to 1.6 when the string in url is not valid. So as long as “master" is in the dropdown, it won’t redirects to v1.6
As far as I'm concerned, that installation widget is now broken. If the purpose of the PR is prevent unsupported versions from showing up then it shouldn't have a customer click down a path of options to have no result.
If the purpose is to "prevent master" then I guess you could update the title and restate the purpose of the PR... then we deal with how poor the CX is now for customers in another issue/pr.
----------------------------------------------------------------
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] [incubator-mxnet] aaronmarkham merged pull request #18587: prevent master version for installation widget
Posted by GitBox <gi...@apache.org>.
aaronmarkham merged pull request #18587:
URL: https://github.com/apache/incubator-mxnet/pull/18587
----------------------------------------------------------------
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