Repository navigation
Conversation
| - `tox -e lint` (includes static type checking) | ||
| - `tox -e unit` | ||
| - Run `tox -e integration` to run the charm integration tests. | ||
| - Run `tox list` to see the available commands. |
There was a problem hiding this comment.
We already mention tox list, which does show the available commands. So it's not too much work for the reader to find the commands. However, LLMs won't necessarily have access to tox, so I think it's better to list the commands.
I'm open to removing the tox list bullet, as the list now covers all the commands. If we remove that bullet, we might as well reformat as a single flat list of commands with short descriptions.
There was a problem hiding this comment.
I'm not sure it's an improvement to stop telling people to just run tox to do everything. With the existing wording, we tell people what they usually want (plain tox) and what it does, how to run the extra tests, and how to get a full list. With the change, we tell people about individual commands instead of to just run them all at once, and they have to run list to find that out.
What's the motivation here? Are humans looking for the individual commands rather than just running everything with tox? Is this for agents, who need a command but don't understand how to run `lists and somehow are missing this from their training data? If it's for agents, then agents.md would be a better place, but I'm still unconvinced about including that in the profiles.
There was a problem hiding this comment.
My 2c: I think it would be cool to go a step further in this direction -- the previous section talks about charmcraft init, but I can see a person or agent landing on "How to manage charms" with an existing charm (perhaps one that uses a different command runner).
This would be a good spot to list the standard charm development commands (format, lint, unit, integration), and then say something like "The Charmcraft profile sets these commands up with tox (perhaps linking to tox docs). Run tox -e <cmd> or tox list to see all available commands.".
There was a problem hiding this comment.
I completely forgot that plain tox will run several commands and was misinterpreting the doc! Thanks for pointing that out @tonyandrewmeyer. I never use tox that way myself. I'll have another think about whether any changes are needed - with @james-garner-canonical's comments in mind too.
There was a problem hiding this comment.
Oh I also forgot that tox does that! Whereas just and make run the first recipe/target (which might be a command that runs other commands, equivalently to tox's envlist, but is often a help command).
There was a problem hiding this comment.
It does literally say "Run tox" 😄. We are going to have to figure out whether a plain make is like a plain tox or a help of some kind (I think we have a mix across repos).
It seems to me that running "all the quick checks", tox by itself, is the best thing to introduce people to. But maybe I'm biased because that's what I would always do unless I was looking to run a single unit test or something like that. It's also what I would want an agent to do if there was no pre-commit forcing (essentially) the same thing.
We have a 'you must have a 'format', 'lint', 'unit', and 'integration' command/environment' rule for public listing, but it doesn't come from a best practice note (and there's the spec that requires these). I think if we're going to have something about that in here, we're going to have to say something about both tox and Make being acceptable, and annoyingly should say that tox is preferred (because our rule is that whatever charmcraft init gives you is preferred).
There was a problem hiding this comment.
Thanks for the discussion. I'm going to close the PR as not a priority. Let's come back to this as part of #2479.
|
The spelling check failure is unrelated. I'll investigate that before merging. |
How-to manage charms has a section that mentions tox for formatting, lining and unit tests, but doesn't show the actual commands. This PR lists the commands to save the reader from having to hunt for them elsewhere.
Preview doc