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

Use short array syntax #318

Merged
merged 8 commits into from Sep 2, 2019

Conversation

@inverse
Copy link
Contributor

commented Aug 17, 2019

Since the minimum version was bumped last year to 5.6 we should leverage some of the niceties that we have.

@inverse inverse closed this Aug 17, 2019

@inverse inverse deleted the inverse:short-array branch Aug 17, 2019

@inverse inverse restored the inverse:short-array branch Aug 17, 2019

@inverse inverse reopened this Aug 17, 2019

DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
@lyrixx
Copy link
Member

left a comment

Thanks, but some code should be updated.
I think we should have the same number of addition than the number of deletion (about git)

DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
Tests/DependencyInjection/MonologExtensionTest.php Outdated Show resolved Hide resolved
Tests/DependencyInjection/MonologExtensionTest.php Outdated Show resolved Hide resolved
inverse added 4 commits Aug 28, 2019
inverse added 2 commits Sep 1, 2019
@inverse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

@lyrixx @stof diff now matches on count. There are 2 comments regarding current things that don't fix style. Shall those be changed too?

@lyrixx
lyrixx approved these changes Sep 2, 2019
Copy link
Member

left a comment

👍 Thanks for working on this

@Seldaek Seldaek merged commit a39d60a into symfony:master Sep 2, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@inverse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 2, 2019

@lyrixx + @Seldaek thanks for your time in reviewing this and getting it in. I noticed it was merged not squashed. Is this normally how changes are brought to master?

It might look a bit messy in the commits with messages like "Fix up". Are you open to using squash when merging?

@Seldaek

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

I sometimes do it when things get out of control.. Here it could have been done, but I didn't notice. Not that big a deal really.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.