add test to enforce helptext on commands #2648
Conversation
|
Thank you! |
|
pretty excited about this |
This is the best place for inserting it that I found. Test in #2648 should be modified to run `Root.ProcessHelp()`. License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
This is the best place for inserting it that I found. Test in #2648 should be modified to run `Root.ProcessHelp()`. License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
Kubuxu
force-pushed
the
feat/helptext-test
branch
from
e5cad57 to
0529ba9
Compare
|
I rebased and updated this PR with change required by #2694 |
Kubuxu
force-pushed
the
feat/helptext-test
branch
from
0529ba9 to
5e0873e
Compare
|
Before this is merged these commits should be squashed as the latter will be fixing some tests. |
|
There is a test here: https://github.com/ipfs/go-ipfs/blob/master/test/sharness/t0010-basic-commands.sh#L44 |
|
@Kubuxu Might be cool if we could automatically generate the Synopsis. |
|
If you point me to exact style we are aiming for with all possible options I will look into it but it might be a bit hard to do it as I am looking at examples inside current code. |
|
@Kubuxu Something like this? https://github.com/ipfs/go-ipfs/blob/master/core/commands/stat.go#L35 |
|
Problem is that alg wouldn't know where to take |
|
@Kubuxu If it is super hard, I can just do them all manually -- what do you think? |
|
I could introduce something like: |
|
Can you show me a test? This should be automatic for all of them, or we'll constantly be scrambling around updating it. |
|
Like in this case of SynopsisOptionsValues: map[string]string{
"peer": "peerId",
"proto": "protocol",
"interval": "timeInterval",
}And it would be per command basis. |
|
I love it. Would that be hard to implement? |
|
I think not at all. |
|
@RichardLitt see #2785 |
|
@Kubuxu Would it be possible to output a list of all commands that don't have synopses and general helptexts? Something like project-repos? |
|
It is part of the output of the test. |
Kubuxu
force-pushed
the
feat/helptext-test
branch
from
5e0873e to
5ce2bfb
Compare
|
Or run locally: |
|
@Kubuxu Any reason why I would only get a one line output from that command? |
|
Are you running in on this branch or the mater? |
|
Sweet. Ok, here is the result. I think that all of these should be dealt with before this should be closed. Sound right to you, @Kubuxu?
|
|
Yes it has to be dealt with before me merge this as tests have to pass. |
|
Here I go, then. :) |
|
Although the synopsis part is a bit off, IMO. We autogenerate synopses when they arent set. |
whyrusleeping
added
status/deferred
|
@Kubuxu thoughts on rebasing this one, and then marking the test as 'skipped' ? That way we can run it manually as needed, and it doesnt linger in unmerged PR hell forever |
License: MIT Signed-off-by: Jeromy <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
Kubuxu
force-pushed
the
feat/helptext-test
branch
from
5ce2bfb to
a11b806
Compare
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
|
@whyrusleeping done |
This is WIP. Not to be merged for a while.
@RichardLitt here you go :)
License: MIT
Signed-off-by: Jeromy [email protected]