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

Rename doc event #1151

Merged
merged 4 commits into from
Feb 20, 2015
Merged

Rename doc event #1151

merged 4 commits into from
Feb 20, 2015

Conversation

kyleknap
Copy link
Contributor

This PR switches the event_class for the the various operation objects. A big problem with the current event_class attributes was that the values varied widely and were not consistent across help commands.

For example, the values varied from Provider to Operation to an abreviation to a service and even just a straight command for the custom commands.

I switched it so that the event classes rely on a command's lineage for event_class with the names of the commands in the lineage delimited by periods here are some examples:

  • aws ec2 help --> ec2
  • aws s3 cp --> s3.cp
  • aws s3api wait object-exists --> s3api.wait.object-exists

The end event name that bcdoc fires looks a little weird in the end because bcdoc makes assumptions on how the event_class is formatted. I can make a subsequent pull request to fix that.

However by using this format in event_class it does several things for us:

  1. All of the breadcrumbs are fixed. Here are some before and after pictures of some commands:

before (no breadcrumb):
screen shot 2015-02-13 at 3 45 56 pm
after (breadcrumb is there):
screen shot 2015-02-13 at 3 47 14 pm

before (custom commands had weird names):
screen shot 2015-02-13 at 3 48 02 pm
after (name is corrected):
screen shot 2015-02-13 at 3 48 39 pm

before (waiters missed some of the commands):
screen shot 2015-02-13 at 3 49 27 pm
after (all commands are included):
screen shot 2015-02-13 at 3 50 15 pm

  1. Allow us to use :ref: syntax throughout the cli documentation. I added a reference label to the top of
    each reference documentation title. The labels take the form of .. _aws [command] ...:. This allows you to reference the command using the sphynx :ref: syntax which I switched the breadcrumbs to use.

Note that there are 4 commits in this PR. The first two are directly from this PR: #1148. So you do not need to look at those changes for this PR, just for the other PR.

cc @jamesls @danielgtaylor

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 91.86% when pulling 14d4203 on kyleknap:rename-doc-event into 435f655 on aws:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 92.0% when pulling 14d4203 on kyleknap:rename-doc-event into 435f655 on aws:develop.

@kyleknap
Copy link
Contributor Author

Coverage report looks fine despite comment

@jamesls
Copy link
Member

jamesls commented Feb 18, 2015

I think we should get the bcdoc event names updated before merging this. I'd rather avoid having changes such as: https://github.com/aws/aws-cli/pull/1151/files#diff-10d9faf99f1b18c865fd1aebf80d163bR26 which should be unnecessary once we fix the bcdoc event names.

This adds reference labels to top of all reference titles and switched
the bread crumbs to rely on these references.
I will help prevent collisions with other references in the docs
@kyleknap
Copy link
Contributor Author

So these tests will fail until this bcdoc PR is merged in: boto/bcdoc#33

In addition, I made a change to how I do the bread crumbs and references at the top of section headers. I made the change such that they are prefixed by cli:. This helped avoid collisions with references created from docs in the JSON model. There were a few aws <servicename> references in the models.

Otherwise it is ready to be looked at for both PR's.
@jamesls @danielgtaylor

@jamesls
Copy link
Member

jamesls commented Feb 20, 2015

:shipit: Let's get a clean run through travis after the bcdoc merge. Otherwise looks good.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 91.86% when pulling 7add479 on kyleknap:rename-doc-event into 768b2a8 on aws:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 92.0% when pulling 7add479 on kyleknap:rename-doc-event into 768b2a8 on aws:develop.

@kyleknap
Copy link
Contributor Author

Test pass with bcdoc merge.

Coverage stayed the same or increased for files I had changed. Only the s3 tasks.py decreased, which has been happening for most cli pull request, and my changes should not have affected it at all.

Merging otherwise.

kyleknap added a commit that referenced this pull request Feb 20, 2015
@kyleknap kyleknap merged commit 4b0646b into aws:develop Feb 20, 2015
@kyleknap kyleknap deleted the rename-doc-event branch February 20, 2015 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants