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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies #61

Closed
wants to merge 5 commits into from

Conversation

studioromeo
Copy link

@studioromeo studioromeo commented Oct 3, 2019

Hey 馃憢

I've updated the dependencies to their latest available versions.

Theres two main bumps here:

  • PHPUnit 4.x -> 7.5
  • php-cs-fixer 1.x -> 2.x

I've tried to port as much of the cs fixer config over but I noticed that it's reordered the comparison operators in some places. I hope this is ok but let me know if you'd like anything changed.

@studioromeo
Copy link
Author

Heya. Sorry to bump this but I'm wondering as it's been a while if you've had any thoughts on this PR?

I noticed #66 has been raised since I raised this one so I've adjusted this PR to meet the discussions being held in that one.

Thanks

@pyrech
Copy link
Owner

pyrech commented Oct 17, 2019

Hello @studioromeo. Sorry I completely forgot your PR 馃槙 Thanks a lot for it!

I just merged #66 so could please rebase your branch on top of master and resolve the conflict? 馃檹 Thanks

@@ -98,8 +99,6 @@ public function test_it_is_registered_and_activated()
$this->addComposerPlugin($plugin);

$this->assertSame([$plugin], $this->composer->getPluginManager()->getPlugins());
$this->assertAttributeInstanceOf('Composer\IO\IOInterface', 'io', $plugin);
$this->assertAttributeInstanceOf('Pyrech\ComposerChangelogs\Outputter', 'outputter', $plugin);
Copy link
Owner

@pyrech pyrech Oct 21, 2019

Choose a reason for hiding this comment

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

Why did these 2 asserts disappeared?

Copy link
Author

Choose a reason for hiding this comment

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

Ah we may be able to add them back in again but essentially in PHPUnit 8.x assertAttributeInstanceOf is deprecated. Apparently the author felt that allowing tests that check the attribute value is bad practice and so removed them sebastianbergmann/phpunit#3339

I figured here theres little to be gained by adding getters for io & outputter but can do that if you like?

Copy link
Owner

Choose a reason for hiding this comment

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

I think I added those asserts to ensure the plugin is always activated correctly (as some of its dependencies depends from Composer) : https://github.com/pyrech/composer-changelogs/blob/master/src/ChangelogsPlugin.php#L59

But anyway, we can probably drop them 馃槈

@pyrech
Copy link
Owner

pyrech commented Apr 27, 2020

Sorry @studioromeo for not taking enough time to address your comment and merge your PR. In the meantime, #68 was just merged so I guess we can now close this PR.

Again sorry and do not hesitate to reopen if you think we missed something.

@pyrech pyrech closed this Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants