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

using nature sort secrets and configs in cli #307

Merged

Conversation

allencloud
Copy link
Contributor

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

- What I did

  1. sort secrets and configs in cli

- How I did it

- How to verify it

- Description for the changelog

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

@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch 3 times, most recently from ba45d68 to 314fe55 Compare July 7, 2017 08:31
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

@dnephin
Copy link
Contributor

dnephin commented Jul 7, 2017

Although some tests do need to be updated

@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch 2 times, most recently from a569689 to c073bb9 Compare July 7, 2017 16:59
@aaronlehmann
Copy link
Contributor

I think this should use the "natural sort" like #302. ping @ripcurld0

@allencloud
Copy link
Contributor Author

Thanks for your feedback, I will do a change. @aaronlehmann
In addition, @dnephin , I think this PR fails to trigger the CI.

@dnephin
Copy link
Contributor

dnephin commented Jul 11, 2017

hmm, not sure why that is. Maybe cirlceci was having issues?

Have you setup circleci on your fork? I think that can cause problems sometimnes

@yuexiao-wang
Copy link
Contributor

@allencloud This PR needs to be rebased

@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch from c073bb9 to 45e2ae1 Compare July 13, 2017 09:51
Copy link
Contributor

@boaz0 boaz0 left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch 2 times, most recently from 22c7fb8 to dea4565 Compare July 19, 2017 02:00
@aaronlehmann
Copy link
Contributor

CI is failing

cli/command/config/ls_test.go:80: use of package assert without selector
cli/command/config/ls_test.go:81: use of package assert without selector

@allencloud
Copy link
Contributor Author

Thanks for your feedback. @aaronlehmann
I will turn to this PR in few days. Sorry for the delay. ☀️

@vdemeester
Copy link
Collaborator

@allencloud ping 👼

@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch 7 times, most recently from 996824e to 886ebbc Compare September 25, 2017 04:01
@allencloud allencloud changed the title sort secrets and configs in cli using nature sort secrets and configs in cli Sep 25, 2017
@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch from 886ebbc to 5105e0e Compare September 25, 2017 04:04
@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch 3 times, most recently from d25f04b to 48398ee Compare September 25, 2017 05:10
Signed-off-by: Allen Sun <shlallen1990@gmail.com>
@allencloud allencloud force-pushed the sort-secrets-and-configs-in-cli branch from 48398ee to 26f06c5 Compare September 25, 2017 05:18
@codecov-io
Copy link

codecov-io commented Sep 25, 2017

Codecov Report

Merging #307 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #307      +/-   ##
==========================================
+ Coverage   49.22%   49.26%   +0.04%     
==========================================
  Files         200      200              
  Lines       16453    16465      +12     
==========================================
+ Hits         8099     8112      +13     
+ Misses       7934     7933       -1     
  Partials      420      420

@allencloud
Copy link
Contributor Author

Sorry for the delay. And I have updated this PR to use natural sort to sort docker cli config/secret ls output. PTAL @dnephin @aaronlehmann @vdemeester

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@vdemeester vdemeester merged commit bd6e175 into docker:master Sep 25, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.10.0 milestone Sep 25, 2017
@allencloud allencloud deleted the sort-secrets-and-configs-in-cli branch September 25, 2017 07:42
jeanlaurent pushed a commit to jeanlaurent/cli that referenced this pull request Sep 25, 2017
…s-in-cli

using nature sort secrets and configs in cli
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.

9 participants