Skip to content

Updated the styles of the container commands #15983

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

Closed

Conversation

javiereguiluz
Copy link
Member

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

This PR uses comment() which hasn't been merged yet. WIP PR at #15964

This command defines a lot of options, so I don't know if I've updated all the possible outputs:

container_params

container_tags

container_one_service

container_select_service

container_one_parameter

container_tagged_services

@fabpot
Copy link
Member

fabpot commented Sep 28, 2015

What finishing #15964? That would allow me to merge some of the PRs.

@fabpot
Copy link
Member

fabpot commented Sep 28, 2015

fabbot has some good suggestions

@javiereguiluz
Copy link
Member Author

Fixed fabbot issues.

@javiereguiluz javiereguiluz force-pushed the update_container_commands branch from 892ed21 to 7d1deb1 Compare September 30, 2015 06:54
@fabpot
Copy link
Member

fabpot commented Oct 1, 2015

Can you rebase as tests were badly broken where run? Thanks.

@javiereguiluz javiereguiluz force-pushed the update_container_commands branch from 7d1deb1 to e93f6da Compare October 1, 2015 13:48
@javiereguiluz
Copy link
Member Author

Rebased.

@fabpot
Copy link
Member

fabpot commented Oct 1, 2015

There are some failing tests.

@javiereguiluz
Copy link
Member Author

I've fixed the tests (at least locally).

}
$tableRows[] = array('Tags', $tagInformation);

$tableRows[] = array('Scope', $definition->getScope());
Copy link
Member

Choose a reason for hiding this comment

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

You should pass false to getScope to avoid the deprecation notice.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks.

@fabpot
Copy link
Member

fabpot commented Oct 1, 2015

Thank you @javiereguiluz.

@fabpot fabpot closed this in 238419f Oct 1, 2015
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.

3 participants