Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Generate reference from snapcraft's "help" command. #318
Conversation
|
This is just a first cut to get an idea of you generally like the idea. Having submitted this I wonder if I shouldn't generate just one docs/reference.md file with an auto-generated ToC instead... |
|
Ah... maybe I should explain the rationale for this: it'd be great if (once the md importer for developer.u.c finally lands) we could show an automatically updated reference without having to duplicate this information. |
|
I changed it to only write to one file. Let me know if you want me to break out the functionality into a separate file outside |
dholbach
changed the title from
First cut of generating help from snapcraft's "help" command.
to
Generate reference from snapcraft's "help" command.
Feb 11, 2016
sergiusens
reviewed
Feb 15, 2016
| + self._write_index_doc() | ||
| + | ||
| + def _write_index_doc(self): | ||
| + text = ''' |
sergiusens
Feb 15, 2016
Collaborator
Use three """ instead, also, maybe call this text_template or something... @elopio will have ideas wrt to consistency on this one.
|
I am thanks for this improvement!!!! |
elopio
reviewed
Feb 15, 2016
| +def _get_help_output(topic, lines=False): | ||
| + TRANSLATE = { | ||
| + 'tar-content': 'tar_content', | ||
| + } |
elopio
Feb 15, 2016
Member
Does this apply to all the topics with a dash? If so, you could just replace the dash.
dholbach
Feb 16, 2016
Contributor
Right now it's the only one and I thought I'd rather make this explicit than just changing anything which comes up with a dash.
elopio
reviewed
Feb 15, 2016
| + | ||
| + | ||
| +def _run_snapcraft(cmd, lines=False): | ||
| + cmd = ['./bin/snapcraft']+cmd |
elopio
Feb 15, 2016
Member
pep8 doesn't check this file, which is probably a bug. You should use spaces between the operators:
cmd = ['./bin/snapcraft'] + cmd
elopio
reviewed
Feb 15, 2016
| +def _run_snapcraft(cmd, lines=False): | ||
| + cmd = ['./bin/snapcraft']+cmd | ||
| + command = subprocess.Popen(cmd, stdout=subprocess.PIPE) | ||
| + output = command.communicate()[0] |
elopio
reviewed
Feb 15, 2016
| + output = command.communicate()[0] | ||
| + | ||
| + if sys.version_info.major == 3: | ||
| + output = output.decode('utf-8') # bytes to str |
elopio
Feb 15, 2016
Member
I think I would prefer just to ignore python2. @sergiusens do you know if there's a good reason to keep this check?
dholbach
Feb 16, 2016
Contributor
We might run this on developer.u.c where I don't know which python versions are available.
elopio
reviewed
Feb 15, 2016
| + | ||
| + def run(self): | ||
| + assert os.getcwd() == os.path.abspath(os.path.dirname(__file__)), \ | ||
| + 'Must be run in snapcraft root.' |
elopio
Feb 15, 2016
Member
This might be confusing. Root is also the user. So maybe:
'Must be run in the root directory of snapcraft.'
|
I like the idea. I don't like that it's in the setup.py. |
snappy-m-o
commented
Feb 16, 2016
|
Can one of the admins verify this patch? |
snappy-m-o
commented
Feb 16, 2016
|
Can one of the admins verify this patch? |
snappy-m-o
commented
Feb 16, 2016
|
Can one of the admins verify this patch? |
|
I hope it's all fine now. |
|
Looks good. Your replies make a lot of sense, and thanks for moving it. |
|
It's fixed now. |
|
add to whitelist |
|
What does "add to whitelist" mean? Do I need to do anything? |
|
@dholbach I think he's experimenting with the bot. |
|
retest this please |
snappy-m-o
commented
Feb 23, 2016
|
Can one of the admins verify this patch? |
|
landing... |
added a commit
that referenced
this pull request
Feb 23, 2016
elopio
merged commit c76afe7
into
snapcore:master
Feb 23, 2016
dholbach
deleted the
dholbach:add-doc-command
branch
Feb 24, 2016
pushed a commit
to smoser/snapcraft
that referenced
this pull request
Sep 14, 2016
pushed a commit
to kalikiana/snapcraft
that referenced
this pull request
Apr 6, 2017
dholbach commentedFeb 11, 2016
Run:
or
LP: #1544540