Skip to content

Test on Travis always high and low deps #584

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

Merged
merged 9 commits into from
May 12, 2016

Conversation

goetas
Copy link
Collaborator

@goetas goetas commented May 10, 2016

  • before this PR, dependencies of this project were uncertain. this tries to make more clarity on what are the minimum requirements for the serializer
  • added code coverage on scrutinizer
  • performance improvements

@@ -6,6 +6,10 @@ cache:
directories:
- $HOME/.composer/cache

env:
- STABILITY=''
- STABILITY='--prefer-lowest'
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be much better to run a single job for prefer-lowest instead of one per PHP version, to limit the size of the build matrix. Travis allows only 5 concurrent jobs per repo owner (i.e. for all repos of @schmittjoh in this case) so the size of the matrix has an impact

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i tried to to optimize things to lower the build time and now is almost half time.
do you know some projects using the approach you suggested? to see exactly what do you mean

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@schmittjoh
Copy link
Owner

schmittjoh commented May 10, 2016

I'm really not sure about this approach. Previously with the lock file, we have a stable build and if we update the lock, and something breaks we know which dependencies to look at.

Without the lock file, things might break without us changing the code. People will complain about tests already being broken when opening pull-requests and we end up hunting down where it comes from without knowing for which dependencies the tests worked. This highest lowest seems useful before a release, but also very fragile otherwise.

Can we combine both approaches (lock file for normal usage, highest-lowest before a release)?

@goetas
Copy link
Collaborator Author

goetas commented May 10, 2016

Without the lock file, things might break without us changing the code.

personally i prefer to do not commit lock file. Not committing it allows to understand if the declared dependencies and requirements allows to work correctly with the serializer. if some external dependency broke the serializer, the serializer should declare to be conflicting with that version.
committing the lock file ensure that the dependencies at that point in time were ok, but says nothing about the dependencies of the serializer.
A good point about this is #578 (comment) and composer/composer#2461

@stof
Copy link
Contributor

stof commented May 10, 2016

@schmittjoh with the previous lock file, you were testing the library only against dependencies that none of your user would use as you were locking versions unmaintained since more than 2 years. so this was making the testsuite useless (as it would not ensure that things work in a real project) rather than making them fragile.

@schmittjoh schmittjoh merged commit 5c03a86 into schmittjoh:master May 12, 2016
@goetas goetas deleted the versions branch May 14, 2016 09:35
@goetas goetas added this to the v1.2 milestone Aug 3, 2016
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.

3 participants