-
Notifications
You must be signed in to change notification settings - Fork 7
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
Address 2.0 website issues. #224
Conversation
…ror messages clearer, put submission example on page
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.
Some minor changes/clarifications, but at a higher level: I'm not sure if it makes sense to keep the deep dive tutorial the way it currently is structured, which (even if we're not actually making the submission) is geared towards creating a duplicate of an existing model under a different identifier. We could maybe just drop the identifier changing activities and have it be just a walkthrough of the example submission?
Co-authored-by: Katherine Fairchild <[email protected]>
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.
Unfortunately I don't have as much time as I'd like to spend on this review, so while it generally looks good to me, I'm holding out approval for now and hoping a few other sets of eyes can make up the difference!
benchmarks/templates/benchmarks/tutorials/tutorial_deepdive_2.html
Outdated
Show resolved
Hide resolved
benchmarks/templates/benchmarks/tutorials/tutorial_deepdive_2.html
Outdated
Show resolved
Hide resolved
benchmarks/templates/benchmarks/tutorials/tutorial_deepdive_3.html
Outdated
Show resolved
Hide resolved
benchmarks/templates/benchmarks/tutorials/tutorial_quickstart.html
Outdated
Show resolved
Hide resolved
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.
addressed all changes
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 some reason Git was showing an unresolved issue when there was none, so I had to dismiss your previous review @mschrimpf, but all your changes were indeed addressed
Addressed the issues I am tagged under in here. As this PR is in web and 431 is on vision, I cannot PR onto it directly.
Specifically, this PR: