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

lxc: add page #1371

Merged
merged 8 commits into from
May 16, 2017
Merged

lxc: add page #1371

merged 8 commits into from
May 16, 2017

Conversation

damianpeterson
Copy link
Contributor

Confusingly, lxd uses lxc as its command. Also, I wasn't sure from the contributing guide how to format optional parameters so have gone with linux standard. I'll remove them if it's not the done thing.

@CLAassistant
Copy link

CLAassistant commented May 9, 2017

CLA assistant check
All committers have signed the CLA.

@agnivade agnivade changed the title add lxc documentation lxc: add page May 9, 2017
@agnivade agnivade added the new command Issues requesting creation of a new page or PRs adding a new page for a command. label May 9, 2017

- List all containers:

`lxc list [{{remote}}:][{{filter}}]`
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand. Which of the possible combinations of these parameters are valid? Can one use either, both, or only one or the other? Or does one of them require the other, but not the other way around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This means you can use any one of:
lxc list to list all containers locally without a filter
lxc list my-remote: to list all containers on the my-remote private remote
lxc list abc to list all local containers matching the filter, abc
or
lxc list my-remote:abc to list all containers on the my-remote private remote matching abc

They must be in the order as written though as the value proceeding the colon is the remote.

Using the square brackets to denote optional parameters or arguments is pretty standard in Linux docs but I'm not sure how I should format this in tldr land. I'm also aware that tldr shouldn't have lengthy examples. Any advice gratefully received.

Copy link
Member

Choose a reason for hiding this comment

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

Using square brackets is perfectly fine.

@@ -0,0 +1,31 @@
# lxc

> Manages Linux containers (using the lxd REST API).
Copy link
Member

Choose a reason for hiding this comment

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

We normally use the infinitive tense. Please change "Manages" --> "Manage".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Cheers.


- List all containers:

`lxc list [{{remote}}:][{{filter}}]`
Copy link
Member

Choose a reason for hiding this comment

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

What does "remote" and "filter" mean 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.

See above reply for more detail but essentially your containers can be stored locally or on a remote server. If you just lxc list you'll see all your local containers. The filter can be used to narrow your search down.

@waldyrious
Copy link
Member

@damianpeterson thanks for the PR! We've left some inline comments, please take a look.

@damianpeterson
Copy link
Contributor Author

Looking through other people's page files it seems that no one else is using optional parameters. Perhaps I should give only specific examples for tldr? And clarify in the command description instead?

@sbrl
Copy link
Member

sbrl commented May 9, 2017

@damianpeterson Thanks for the PR! Personally, I'd err on the side of caution and add them anyway (without the square brackets). It's like the -a vs --argument-name thing, I think. @waldyrious, @agnivade, what do you think?

sbrl
sbrl previously requested changes May 9, 2017

- List all containers (does not work on public remotes):

`lxc list [{{private-remote}}:][{{filter}}]`
Copy link
Member

Choose a reason for hiding this comment

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

Is a private-remote an IP address? What's a filter? I think that a specific example here might make all the difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A remote is an alias to a URL. One of the first things you learn do is lxc launch ubuntu:16.04 which creates a new container locally from the default ubuntu remote (that's mapped to https://cloud-images.ubuntu.com/releases) and pulls down the most recent 16.04 image. You can make your own remotes and list all that you know about but that might add too many examples for tldr.

@damianpeterson
Copy link
Contributor Author

damianpeterson commented May 9, 2017

How about something like:

- List all local containers:

`lxc list`

- List all local containers that match a search string (i.e. `python`):

`lxc list python`

- List all containers on a private remote (i.e. `production:`) that match a search string (i.e. `python`):

`lxc list production:python`

The problem with this is that we'll only be able to fit a couple of commands in and some of the others are quite useful. I guess the idea behind tldr is to remind you of common commands you struggle to memorize and folks can always go to the man pages if they want more details.

It would be kinda nice if we had some agreed way to denote an optional argument or parameter though.

(as a side note, if you want to try lxd/lxc online https://linuxcontainers.org/lxd/try-it/ is a pretty awesome resource)

@sbrl
Copy link
Member

sbrl commented May 10, 2017

Yeah, that might work. You've got the general idea right there! Take the eyeD3 page I added recently - the last example is one that I personally have a hard time remembering, so I added it to my tldr page 😺

We try to order the commands via natural progression too - i.e. in the order that you're likely to want to execute them.

@damianpeterson
Copy link
Contributor Author

@sbrl how about this version? I've added a line to the heading that describes how the remote works and that it's always optional. That way we don't have to double the number of examples. What do you think?

@sbrl
Copy link
Member

sbrl commented May 11, 2017

I'm a bit out of my depth here. @agnivade, @waldyrious - thoughts?

@sbrl sbrl dismissed their stale review May 11, 2017 10:01

I'm a bit out of my depth here.

Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

LGTM overall. Just put a minor comment.

I am okay if you want to bring back the square brackets. Just clarify that if the remote is not mentioned, then it falls back to querying locally.


- List local containers matching a filter string. Omit the filter to list all local containers:

`lxc list {{filter}}`
Copy link
Member

Choose a reason for hiding this comment

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

I think "filter" can be better expressed as "match_string"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done.

@damianpeterson
Copy link
Contributor Author

Nice one. It's good to be able to use square brackets for optional parameters. Done. Thanks.

# lxc

> Manage Linux containers using the lxd REST API.
> 'remote:' refers to the alias for a remote server (i.e. 'ubuntu:') and when omitted will apply locally.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the (i.e. 'ubuntu:') is particularly elucidative here. In fact, the whole sentence is longer than I'd be comfortable with. Maybe I'd write this as:

Any container names or patterns can be prefixed with the name of a remote server.

Does that sound OK to you guys?

Copy link
Member

Choose a reason for hiding this comment

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

This does not mention that by default, it applies to the local machine.

> Manage Linux containers using the lxd REST API.
> 'remote:' refers to the alias for a remote server (i.e. 'ubuntu:') and when omitted will apply locally.

- List local containers matching a string. Omit string to list all local containers:
Copy link
Member

Choose a reason for hiding this comment

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

I prefer writing "omit the string" to make this extra clear :)


`lxc list {{match_string}}`

- List images matching a string. Omit string to list all images:
Copy link
Member

Choose a reason for hiding this comment

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

Here too: "Omit the string".

@waldyrious
Copy link
Member

Hey all -- sorry for not replying earlier, I had a bit of a rough week. I don't feel comfortable delaying this PR further with discussion about the optional parameter notations, so I'll just add this as a side note which should not affect the mergeability of the PR.

That said: as @damianpeterson pointed out, if indeed we don't have optional parameters elsewhere, then using them here is fine, but not perfectly fine IMO. We do have the downside of having to compact extensive commands because of the need to show the various combinations, but the way we've handled them in the past is by creating pages for the subcommands. In this case it may not be justified if the syntax actually doesn't change that much among subcommands (please correct me if I'm wrong @damianpeterson).

So we have three solutions here:

  1. Use only explicit examples with matching descriptions, showing one or two with the remote prefix to demonstrate how it works. This has the advantage of making the page simpler to read and understand, and more explicit, but at the cost of including less examples than would be sufficient for tldr-pages's purposes of a providing a sufficiently complete quick-start or refresher. I guess I'd trust @damianpeterson to make the decision of whether this trade-off is justified, were we to pick this option
  2. Split the page into multiple ones, one for each subcommand. Again, it would make sense for the PR author to weigh in on whether this makes sense -- i.e. whether the subcommands are different enough, and extensive enough to justify having separate pages, and whether the level of detail such a division would allow is compatible with the scope of tldr-pages (keeping in mind the pages for other complex commands, say grep, less or vim.
  3. keep the current form of the PR as the final one (with minor tweaks per the inline comments), and from now one agree to using square brackets to denote optional parameters in tldr pages if justified. This has the downside of making the syntax richer for human readers, resulting in potentially more complex pages (which I fear will be abused in a way that would compromise the project's goals), but on the other hand, it does conform to common syntax for denoting command line examples. If we're to go ahead with this option, I ask that we at least agree to use square brackets sparingly, to prevent the slippery slope of making the tldr-pages exceedingly complex in an attempt to cram in as much functionality of the commands as possible

Of the three options above, my preference would be 1, then 2, then 3, but as I said, this note is not meant as a review of this particular PR -- we could merge it as-is since it is already in a form that would clearly bring a net benefit to the project, for little or no downside (i.e. introducing the precedent of square brackets notation).

@agnivade
Copy link
Member

I agree with 1 too. But as @damianpeterson said, he won't be able to contain the useful examples within the limit of 8 that we have. That's why I was OK with adding square brackets. I will leave it to him to see what is feasible.

@damianpeterson
Copy link
Contributor Author

damianpeterson commented May 14, 2017

@waldyrious I think that it would be a bit much to have additional pages for each subcommand.

For me, I turn to tldr when I just need a quick reminder and can't be bothered with a full man page. A use case: "What was the command for creating a new container from a standard Linux instance? Was it run or start or create?". If I tldr lxc and see that lxc launch [remote:]image container (and not lxc start...) is the one I'm after, then tldr has done its job. If I have to go to another page then I might as well have used man.

As far as square brackets denoting optional arguments goes, it's standard practice in documentation I come across on a daily basis (all Linux documentation, npm, MDN) and in the case of lxc's examples the remote: option is used in just about every common command and would cause excess examples if we had to split them all. I agree that allowing square brackets could lead to more complex examples but if you ask contributors to only use them when they're commonly used you might avoid the temptation to squeeze in every optional parameter.

I'm also very aware that I'm only one user and new to tldr at that. Totally your call :)

@waldyrious
Copy link
Member

Fair enough, @damianpeterson, I can agree that in this case, using the square brackers might be the most sensible approach. Thanks again for the contribution :)

@agnivade agnivade merged commit 9fbc74e into tldr-pages:master May 16, 2017
@Managor Managor mentioned this pull request Jan 19, 2025
71 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new command Issues requesting creation of a new page or PRs adding a new page for a command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants