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.

Sign up for GitHub

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

Jump to bottom

Refactor monster Configuration class. #660

Merged
merged 1 commit into from
May 10, 2018
Merged

Refactor monster Configuration class. #660

merged 1 commit into from
May 10, 2018

Conversation

jwage
Copy link
Member

@jwage jwage commented May 8, 2018 •
edited
Loading

Fixes #544

  • Extract functionality out of Configuration in to separate classes and proxy to the new classes initially.
  • Extract MigrationRepository::getMigrationsToExecute method to a MigrationPlanCalculator class.
  • Move resolveVersionAlias to VersionAliasResolver class.
  • We need some kind of dependency/object factory. Right now all our objects and dependencies are in Configuration class and it is being used as a sort of service locator.

@jwage jwage added this to the 2.0 milestone May 8, 2018
@jwage jwage self-assigned this May 8, 2018
@jwage jwage force-pushed the configuration-refactor branch 7 times, most recently from 3e1fc0d to 3073463 Compare May 10, 2018 07:35
@jwage jwage requested review from Majkl578 and stof May 10, 2018 07:40
jwage
jwage commented May 10, 2018
View reviewed changes
lib/Doctrine/Migrations/MigrationRepository.php Outdated
/** @return Version[] */
public function getMigrationsToExecute(string $direction, string $to) : array
{
$migrationPlanCalculator = new MigrationPlanCalculator($this);
Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops. This needs to move to DependencyFactory.

@jwage jwage force-pushed the configuration-refactor branch from 3073463 to f8f7727 Compare May 10, 2018 13:10
@jwage jwage changed the title [WIP] Refactor monster Configuration class. Refactor monster Configuration class. May 10, 2018
@jwage
Copy link
Member Author

jwage commented May 10, 2018

If anyone has any feedback, leave comments on this PR and I will open up new PRs to address the issues.

@jwage jwage merged commit 3685563 into master May 10, 2018
@jwage jwage deleted the configuration-refactor branch May 10, 2018 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Majkl578 Majkl578 Awaiting requested review from Majkl578

@stof stof Awaiting requested review from stof

Assignees

@jwage jwage

Labels
Improvement
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.

Review responsibilities of the Configuration object
1 participant
@jwage