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

Update composer lockfile to fix warning #731

Merged
merged 2 commits into from Jul 11, 2019
Merged

Conversation

feliks-montez
Copy link
Contributor

@feliks-montez feliks-montez commented Jul 11, 2019

The Problem

When running composer install, we get 3 warnings:

  • Warning: The lock file is not up to date with the latest changes in composer.json. You may be getting outdated dependencies. Run update to update them.
  • Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
  • Package silex/silex is abandoned, you should avoid using it. Use symfony/flex instead.

Fixes

I have run composer update to update composer.lock to reflect composer.json and the first warning has gone away.

phpunit-mock-objects has been absorbed into phpunit, so I am requiring phpunit version 8.0 or greater in composer.json. In changing this, I had to upgrade some deprecated PHP syntax to be PHP >7.2 compliant and modify the phpunit.xml configuration file.

Tests

After making all of these changes, the PHP unit tests are now all passing on PHP 7.3 without any warnings:
passing-php-unit-tests

All e2e tests are passing except the following three, which @Andrewdt97 fixed in a currently unpublished PR. These three must have started as a result of the autosaving feature added in #724, but for some reason, they have not failed until now.
composer-update-e2e-error

So test-wise, this PR is good to go.

Persisting Issues

When running a composer install, this message still pops up:
Package silex/silex is abandoned, you should avoid using it. Use symfony/flex instead.

In order to get fix this, we will need to migrate from the Silex micro-framework to Symfony 4 / Flex. Here is a short post with some suggestions on how to migrate. I have created a separate trello card for this task since it is fairly involved.


This change is Reviewable

@megahirt
Copy link
Collaborator

After addressing the known issues, we want to confirm that both before and after this change, PHP and E2E tests are all passing. And this should be stated on the PR message.

@feliks-montez feliks-montez marked this pull request as ready for review July 11, 2019 08:30
@feliks-montez
Copy link
Contributor Author

1 similar comment
@megahirt
Copy link
Collaborator

Copy link
Collaborator

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

Great! It looks like maybe you unintentionally downgraded silex? A minor point but will wait until this is addressed...

Reviewed 48 of 48 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @feliks-montez)


src/composer.json, line 27 at r2 (raw file):

    "ocramius/lazy-property": "dev-feature/factory",
    "ramsey/uuid": "~3.0",
    "silex/silex": "~1.3.2",

This doesn't look right - we shouldn't touch the silex dependency in this PR, correct?


test/php/phpunit.xml, line 13 at r2 (raw file):

    convertWarningsToExceptions="true"
    forceCoversAnnotation="false"
    mapTestClassNameToCoveredClassName="false"

just wondering what this does and we we're removing it?

includes updating to phpunit 8
Copy link
Contributor Author

@feliks-montez feliks-montez left a comment

Choose a reason for hiding this comment

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

Reviewable status: 47 of 48 files reviewed, 2 unresolved discussions (waiting on @megahirt)


src/composer.json, line 27 at r2 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

This doesn't look right - we shouldn't touch the silex dependency in this PR, correct?

Good catch! I'm not sure how that number changed.


test/php/phpunit.xml, line 13 at r2 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

just wondering what this does and we we're removing it?

It actually seems to do nothing anymore, according to this issue:
sebastianbergmann/php-code-coverage#260
It's an invalid property in phpunit 8, so I just got rid of it.

Copy link
Collaborator

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

Wow, very professional and comprehensive PR message - great job!

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @megahirt)

Copy link
Collaborator

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


test/php/phpunit.xml, line 13 at r2 (raw file):

Previously, feliks-montez (Feliks Montez) wrote…

It actually seems to do nothing anymore, according to this issue:
sebastianbergmann/php-code-coverage#260
It's an invalid property in phpunit 8, so I just got rid of it.

ok thanks for the info

@megahirt megahirt merged commit 0d8792b into master Jul 11, 2019
@megahirt megahirt deleted the update-composer-deps branch July 11, 2019 14:26
Nateowami pushed a commit that referenced this pull request Aug 2, 2019
Update composer lockfile to fix warning

Former-commit-id: 0d8792b
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