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

Service logs formatting #31672

Merged
merged 2 commits into from
Mar 13, 2017
Merged

Service logs formatting #31672

merged 2 commits into from
Mar 13, 2017

Conversation

dperny
Copy link
Contributor

@dperny dperny commented Mar 9, 2017

This PR updates #29197 to address the concerns with that PR.

  • Removed the --details flag which isn't supported in the swarmkit API at this time. It may return later.
  • Fixed a possible issue with setting the padding variable on recommendation of @aaronlehmann.
  • Declined to implement a go template formatter. It's a lot of work and too much control; it may return later.
  • Changed no-ids to no-task-ids

Does this look good @aaronlehmann @cpuguy83 @vdemeester ?
/cc @aluzzardi

- Align output. Previously, output would end up unaligned because of
longer task names (e.g. web.1 vs web.10)
- Truncate task IDs and add a --no-trunc option
- Added a --no-ids option to remove IDs altogether
- Got rid of the generic ID Resolver as we need more customization.

Signed-off-by: Andrea Luzzardi <[email protected]>
@dperny
Copy link
Contributor Author

dperny commented Mar 9, 2017

This PR supersedes #29197 and that PR can be closed.

@dperny
Copy link
Contributor Author

dperny commented Mar 10, 2017

I don't believe the test failure is my fault.

@aluzzardi aluzzardi added this to the 17.04.0 milestone Mar 10, 2017
@aluzzardi aluzzardi added the priority/P1 Important: P1 issues are a top priority and a must-have for the next release. label Mar 10, 2017
@aluzzardi
Copy link
Member

Tentatively targeting 17.04.0

Copy link
Member

@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 🦁
Experimental failure is a flaky test yeah 😓

@aluzzardi
Copy link
Member

@cpuguy83 any feedback?

@aluzzardi
Copy link
Member

LGTM

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 13, 2017
@cpuguy83
Copy link
Member

Damnit, hit the wrong button, no confirmations or anything...
@dperny You can just remove the last commit.

@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 13, 2017
@cpuguy83
Copy link
Member

Ok, I fixed it.

@cpuguy83 cpuguy83 merged commit aa733ba into moby:master Mar 13, 2017
@thaJeztah thaJeztah added impact/changelog impact/cli and removed priority/P1 Important: P1 issues are a top priority and a must-have for the next release. labels Mar 13, 2017
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
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.

6 participants