Riak 2.x [JIRA: CLIENTS-128]
Closed this issue · 17 comments
I Didn't notice you guys are developing a rewrite for 2.x.
I started a 2.x fork inspired by the java api,
was planning to do a PR once it my fork is close to be stable
but it looks like we are doing pretty much the same thing.. :(
In same parts my branch seems to be a little ahead
Would you guys be interested in using it ?
Please see :
First glance to the minor things:
- The composer.lock should be part of the repo to ensure everyone is on the same pinned version
- You introduce a requirement for PHP 5.4 (:+1:) but that makes the description of the project inaccurate
- As per the autoloading we might want to adopt PSR-4
I'll have a look to the rest later in the day and also wait for the Basho team to say something
Hey Fabio, thanks for joining the fun! I will add this issue to my sprint that starts today to review your code.
Marco, do you think that the lock file is truly necessary if you limit composer.json to the latest patch versions? They shouldn't really be changing the composer.json unless they know what they are doing anyways.
Thanks @marcodebortoli @christophermancini,
About the composer lock
it is not necessarily recommended for libraries, It is valid only at the root project,
witch means any project including riak-php-client will get the dependencies from riak-php-client/composer.json using the latest version and then create its own my-project/composer.lock
If we add a composer.lock we might be testing against a different version from what users will experience when installing riak-php-client
Thats why we don`t have lock files in projects like doctrine2 or symfony
Please see :
@FabioBatSilva thanks for that insight and the reference links, so basically its a preference thing. Personally, I don't prefer adding it in but I am open to discussion.
@christophermancini yes, as you said it's a matter of preference really.
I generally prefer to commit the lock because it puts developers on the same page and can add a bit of stability and it will avoid the recurrent "it works on my machine"
Pinning to the latest patch will not provide us the ability to be up to date especially with security fixes in our dependencies.
Anyway, either ways are fine by me, I can see value in both approaches.
Well, based on this article, it is starting to sound like committing the lock file is a better approach...
https://blog.engineyard.com/2014/composer-its-all-about-the-lock-file
Thoughts? @FabioBatSilva @marcodebortoli
If I have to vote, I would say to commit it, but as said earlier I'm happy both ways. As @FabioBatSilva mentioned the .lock file is more relevant to the end user's project, but it can be beneficial for us too during development.
IMHO .lock does not make sense for libraries,
As .lock from libraries are ignored it will hide possible failures during the lib development/test
Some one installing riak-php-client might be using a different lib version then what we have on our composer.lock And we will never know something is broken in some dependency until some one experience a bug while using riak-php-client.
@FabioBatSilva the .lock file make sense for us as maintainers if we internally start to rely on 3rd party dependencies. For the end user instead composer will resolves those conflict where possible giving priority to the dependency dependencies ignoring the lock itself as far as I know, unless something changed. I might ask Jordi.
@marcodebortoli According composer docs :
For your library you may commit the composer.lock file if you want to. This can help your team to always test against the same dependency versions. However, this lock file will not have any effect on other projects that depend on it. It only has an effect on the main project.
I think that for libraries the .lock brings more problem then solutions, as user and maintainers might be using a different versions..
It's better if we find the problems during our tests then lib user finding it in prod.
this lock file will not have any effect on other projects that depend on it making it useful for us and useless for users.
Composer itself committed the lock file https://github.com/composer/composer
Anyway, as I said, I'm happy either ways.
I discussed it with the rest of the client team and the consensus is to use the approach that is most common in the community. I viewed about 10 or so packages on packagist under the Popular packages heading and not a single one committed the lock file so I think that's a winner.
Fine with me.
@christophermancini @marcodebortoli I did a couple more tests and refactoring,
Should i send a pull request including what a have so far and continue the development from there ?
Of course :)
@FabioBatSilva my only concern is that I couldn't merge your code into mine because it varies so greatly. I think the best approach would be to identify the components of what you have that offer an improvement over what I have and offer those as a PR. Also, when you create a PR, please target the 2.0.x branch for the destination and not develop or master.
I am nearing a fairly large commit covering objects and CRDTs which I suspect I can probably get pushed up today.
@christophermancini let me known when you push it and I'll rebase my branch against 2.0 and then send the PR.