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

[CS] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general Travis config, upgrade coveralls, and general fixes #1047

Merged
merged 2 commits into from Feb 15, 2018

Conversation

robfrawley
Copy link
Collaborator

@robfrawley robfrawley commented Jan 31, 2018

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

This PR contains a number of 2.0 upkeep items to bring the codebase into a cleaner state, including:

  • Changed AbstractCompilerPass::log() signature to variadic for string replacements; prefixed generated log messages with [liip/imagine-bundle]; and removed legacy behavior required for unsupported Symfony releases.
  • Added some type hints and return types; removed unused class imports; corrected @param and @return docblock values; added @throw docblock entries as required; added explicit class properties for those that were dynamically set prior; cleaned up some simple implementations; and removed unused method-scope variables.
  • Removed DependencyInjection/Factory/ChildDefinitionTrait, a holdover from logic required for unsupported Symfony releases.
  • Removed Form/Type/ImageType::setDefaultOptions() and Form/Type/ImageType::getName(), both legacy behavior for older Symfony releases.
  • Removed any logic specific to unsupported imagine/Imagine releases.
  • Removed tests for removed code (see above); added checks to ensure compiler passes log to the container; added environment-dependant metadata reader compiler pass test.
  • Cleans up the Travis build configuration to be more readable and update dependencies:

As for the last bullet point, was there a reason to include those @sebastianblum? They make the build script more confusing and AFAIK we don't use cron automated builds with this repository on Travis. Is there another reason to have them?

@robfrawley robfrawley added State: Confirmed This item has been confirmed by maintainers as legitimate. Type: Dependencies This item pertains to dependencies of this project. Type: Source Code This item pertains to the source code of this project. Attn: Minor This issue or PR is a minor problem or minor change. labels Jan 31, 2018
@robfrawley robfrawley added this to the 2.0.0 milestone Jan 31, 2018
@robfrawley robfrawley self-assigned this Jan 31, 2018
@robfrawley robfrawley force-pushed the feature-cleanup-and-fixes branch 2 times, most recently from f4279cf to 62c6d9d Compare January 31, 2018 05:44
@robfrawley robfrawley changed the title [CI] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general cleanup [CI] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general cleanup Jan 31, 2018
@robfrawley robfrawley changed the title [CI] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general cleanup [C] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general cleanup Jan 31, 2018
@robfrawley robfrawley changed the title [C] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general cleanup [CS] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general cleanup Jan 31, 2018
Copy link
Collaborator

@alexwilson alexwilson left a comment

Choose a reason for hiding this comment

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

Really nice clean-up 👍

@robfrawley robfrawley changed the title [CS] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general cleanup [CS] [Dependencies] [Dependency Injection] [Form] [General] Update compiler log format, remove legacy code, remove deprecated, fix docblocks/style, general Travis config, upgrade coveralls, and general fixes Feb 15, 2018
@robfrawley robfrawley merged commit d65881b into liip:2.0 Feb 15, 2018
@sebastianblum
Copy link
Contributor

great work @robfrawley thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Attn: Minor This issue or PR is a minor problem or minor change. State: Confirmed This item has been confirmed by maintainers as legitimate. Type: Dependencies This item pertains to dependencies of this project. Type: Source Code This item pertains to the source code of this project.
Projects
2.x Sprint 001
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

3 participants