Skip to content

Conversation

@allencloud
Copy link
Contributor

@allencloud allencloud commented Jul 10, 2017

Signed-off-by: allencloud allen.sun@daocloud.io

This PR adds filter scope for command events, while I think there are still some cluster events missing which are introduced in PR moby/moby#32421.

- What I did

  1. docs: add filter scope for command events, covers one part of missing documentation for swarm events #252;
  2. add more cluster events service, node, secret, config, and remove in network;
  3. add examples;
  4. sort some event actions in lexicographical order.

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@vieux
Copy link
Contributor

vieux commented Jul 11, 2017

LGTM

@allencloud allencloud force-pushed the add-filter-scope-for-events branch from 6c25fee to 8b5e9fc Compare July 12, 2017 06:45
@allencloud allencloud changed the title docs: add filter scope for command events docs: add filter scope for command events and more cluster events Jul 12, 2017
@allencloud
Copy link
Contributor Author

@vieux I updates this PR to add more details in docker events. And these things are described in the PR description. PTAL. Thanks a lot.

@codecov-io
Copy link

codecov-io commented Jul 12, 2017

Codecov Report

Merging #314 into master will decrease coverage by 0.14%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #314      +/-   ##
==========================================
- Coverage   48.83%   48.68%   -0.15%     
==========================================
  Files         186      186              
  Lines       12413    12416       +3     
==========================================
- Hits         6062     6045      -17     
- Misses       5977     5996      +19     
- Partials      374      375       +1

@allencloud
Copy link
Contributor Author

ping @vieux @dnephin @thaJeztah
Could you help to review the content added to the docs about events command? ☀️

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

One nit, but LGTM otherwise

We may want to start thinking if we can present these in a different way, was thinking of a "matrix", but there's a lot of object types that don't share the same event-types, so possibly not a solution

- `remove`
- `update`

#### secrets
Copy link
Member

Choose a reason for hiding this comment

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

Can you use a capital S 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.

Oh, my bad. Updated now. Thanks for the review. ☀️

Signed-off-by: allencloud <allen.sun@daocloud.io>
@allencloud allencloud force-pushed the add-filter-scope-for-events branch from 8b5e9fc to 8639c9b Compare July 15, 2017 01:17
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants