Skip to content
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

install: remove from menu, update in docs (nav and content) #381

Closed
wants to merge 0 commits into from

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Nov 18, 2022

I think it's confusing to promote the idea of direct installation for CML since it's not the happy path, so I'm proposing to clarifying that in the docs, and to remove the big menu button. Other notes for review below...

@jorgeorpinel jorgeorpinel added documentation Markdown files A: website Area: website labels Nov 18, 2022
@jorgeorpinel jorgeorpinel requested a review from a team as a code owner November 18, 2022 06:43
@shcheklein shcheklein temporarily deployed to cml-dev-nav-menu-xcsbllqu3drdg November 18, 2022 06:44 Inactive
@jorgeorpinel jorgeorpinel requested a review from a team November 18, 2022 06:44
Comment on lines 37 to 38
"label": "Install as a Package",
"slug": "install"
},
{
"label": "Command Reference",
"slug": "ref",
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Nov 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Moved this entry one place up. Not a strong opinion, but I think it makes more sense as it's similar to the other lose pages (guides, I guess?).

Comment on lines 5 to 7
Installing CML directly in CI environment is not typically needed, as comes
pre-installed in our provided [Docker Images]. Alternatively, GitHub users can
use the [`setup-cml` action].
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an admon to clarify that you probably don't want to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the changes to this file (and nav) good at least? If you don't want to remove the confusing menu entry.

Otherwise feel free to take this over, close it, etc. Thanks @iterative/cml

Comment on lines 17 to 30
<details>

Instructions for installing [Node.js](https://nodejs.org) and its package
manager `npm` can be found below.
### Installing Node.js

<toggle>
<tab title="GitLab">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hid the 3rd party tool docs (not ideal to maintain, but I see how it can be a useful tip here).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW is this missing a BitBucket tab?

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Nov 18, 2022

Last note: not sure what's the point of the big Download button in the home page:

image

Can we kill it?

@github-actions
Copy link

19b43d8

Link Check Report

All 4 links passed!

Copy link
Contributor

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely a wontfix (CC @dmpetrov)

@shcheklein
Copy link
Member

I think we needed this for a patent application. It probably over and we can remove it safely.

@jorgeorpinel
Copy link
Contributor Author

Didn't quite get any of the feedback TBH 😬

@jorgeorpinel
Copy link
Contributor Author

Oops killed this PR accidentally. Re-done in #447.

@jorgeorpinel jorgeorpinel removed their assignment Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: website Area: website documentation Markdown files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants