zendframework / zend-inputfilter

InputFilter component from Zend Framework
BSD 3-Clause "New" or "Revised" License
64 stars 50 forks source link

remove composer.lock #107

Closed basz closed 8 years ago

basz commented 8 years ago

https://gist.github.com/basz/2575cdb5e6a0dd482ed19b2e4be02696

svycka commented 8 years ago

@basz travis is configured to test locked versions so composer.lock can't be removed

Maks3w commented 8 years ago

Could you add the filename to gitignore?

Maks3w commented 8 years ago

Remove all those locked entries if favor of latest from travis.yml

weierophinney commented 8 years ago

This file is intentionally kept to allow us to do a lowest/locked/latest test strategy.

Maks3w commented 8 years ago

@weierophinney What is the purpose of the locked strategy? When composer.lock should be updated then?

weierophinney commented 8 years ago

@Maks3w: When we discover issues against latest dependencies and want to implement fixes for them.

Maks3w commented 8 years ago

@weierophinney I still not understand. Why not develop against latest versions?

geerteltink commented 8 years ago

Sounds like sort of a backlog. If suddenly the tests fail, I think you can track which dependencies changed. And more importantly, check what changed inside those dependencies. Without the lock file you have no idea what you tested against last time.

Maks3w commented 8 years ago

Time ago we added composer show --installed in travis.yml for log what versions has been installed.

weierophinney commented 8 years ago

@Maks3w The point is to help contributors and maintainers know what is breaking: is it the code their patch is providing, or is something breaking due to changes in an upstream project? If we have the lockfile, that represents the known dependencies. If a later test run shows the "latest" target failing, but not the "locked", we know that a dependency has changes in more recent versions that affect our component, and we can either (a) fix our code to work against them, or (b) change our constraints for the upstream dependency.

We started doing this approach to understand if changes we were making were both backwards compatible with the earliest supported versions of components (v2 versions of event manager and service manager, specifically), and forwards compatible with latest versions. Adding the lock file helped uncover some cases where changes in the v3 versions were causing issues, and we could backtrack and see the ramifications, and whether we needed to fix the v3 versions, or if there were code changes we could make that would allow forwards compatibility.

The approach has already proved itself, and we'll be adopting it across most components soon. This component, in particular, required it due to its usage of the servicemanager and stdlib components, and wanting to ensure compatibility.

Maks3w commented 8 years ago

I understand. Just one more question. Is this the same as if lowest version works and latest does not works?

Lowest version is always fixed because we set the minimum version in composer.json.

basz commented 8 years ago

Sorry 4 the noice

svycka commented 8 years ago

Lowest version is always fixed because we set the minimum version in composer.json

@Maks3w that's not true, what if one of dependancies with are deep in the tree updates their minimum version they can change for us.

Maks3w commented 8 years ago

@svycka Lowest dependencies are always the same no matter what deep they are in the tree.

a require b v1.0.0 require c v0.2.0 even if b v1.0.1 is released with c v0.2.1 the lowest version installed will be v1.0.0 which is conceptually immutable

svycka commented 8 years ago

@Maks3w, an example when lowest and latest failed but locked pass https://travis-ci.org/zendframework/zend-mvc-console/builds/121179985 also I am not a fan of composer.lock in git repository..

Maks3w commented 8 years ago

@svycka It was because the lowver version of zend-mvc was targetting an unstable version.

https://github.com/zendframework/zend-mvc-console/blob/068b67b951e20a2fd502a8b6e409d8c727f826a2/composer.json#L17

This scenarios shouldn't be happen and the lower version should be always a known released version

svycka commented 8 years ago

I didn't said this is same situation but shit happens :D And for example in this situation we had working version.

Maks3w commented 8 years ago

That situation is forced by developer of the root package. We can to rid all those locked builds and composer.lock versions.

weierophinney commented 8 years ago

The reasons for having the lowest/locked/latest strategy are as follows:

These two only require a composer.json, as the former can be accomplished by using the --prefer-lowest flag when installing.

However, there are additional considerations.

When we consider the "latest", one issue that occurs is we have to try and understand when a break occurred. This is hard to do when no composer.lock is available, as the developer has to look through Travis CI jobs to see when the last passing test was, and then look at the composer info output for that job. With a composer.lock available, they have the information available in their checkout.

From a contributor's point of view, there's another issue: when no composer.lock file is present, if they do a composer install and run tests and they fail before they start working on their patch, what should they do? We get a number of issues regularly where this happens, and the result is:

Having the composer.lock available means that they can know that their changes pass against known good dependencies. Yes, their PRs will still fail against Travis, but we, as maintainers, can identify that the issue is with the latest versions of dependencies, and we can then work on patches for those problems separately from the proposed patches in the pull request.

We started using this approach due to development on zend-mvc, where lack of a lock file meant that as dependencies went out-of-scope, developers were unsure of what changes they could confidently and safely make against the current develop branch. Yes, it's a somewhat unique situation — but as I and others were working on the various BC/FC patches, we realized that it wasn't as unique as we thought, and that the approach has general merit.

I understand that not everybody agrees with the approach. However, I've seen far more benefit from it in the few months since we've started introducing it than any possible detriments from doing so. As such, I'd appreciate it if we could focus on getting code patches in place instead of debating the merits of including a lockfile. They've already proven themselves.