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

[WIP] v1.0.0 / v0.14.0 #253

Merged
merged 106 commits into from
Feb 1, 2016
Merged

[WIP] v1.0.0 / v0.14.0 #253

merged 106 commits into from
Feb 1, 2016

Conversation

tedivm
Copy link
Member

@tedivm tedivm commented Aug 9, 2015

Coverage Status Build Status

This PR is for building the v1.0.0 line of Stash. It is pulling together all the remaining changes that are needed before launching the stable API.

  • Make Stash PSR6 Compatible.
  • Review Compatibility with Upstream Dependencies.
  • Remove deprecated constants.
  • Improve Testing.
  • Add PSR-6 interfaces (waiting on submission of PSR-6 to Packagist).
  • Rebuild Stash Documentation Website.
  • Review docblock.

…ties

This is because in PSR6 the “set” and new expiration functions are no
longer persistent to the driver but instead save data in the object
until it is saved.

This change provides backwards compatibility (for now) to make testing
easier, while providing the foundation of the future changes to this
class.
Conflicts:
	tests/Stash/Test/AbstractPoolTest.php
@tedivm
Copy link
Member Author

tedivm commented Dec 8, 2015

@andrerom if you want to give this another review that would be great. I think it's just about ready for beta testing.


* Added `expiresAt` and `expiresAfter` functions to the Item class.

* `getExpiration` to return current datetime when no record exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

this sound strange, I wouldn't expect a unset expiry to mean it has expired (now). As this is currently not part of the PSR interface, maybe rather return null if none is set.
Otherwise expiry should probably have been on the pool->save( $item, $ttl ) method

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have a problem making this return null- it originally returned false, but when getExpiration was added to the PSR they had their version return the current datetime. Now that it's been removed from the standard we can handle it however we want, so I'll put it back to null.

@andrerom
Copy link
Contributor

andrerom commented Dec 8, 2015

did a quick pass and looks ok, I some logic can be moved from item to pool, but that is a design decision you are free to take afaik.

@tedivm
Copy link
Member Author

tedivm commented Dec 14, 2015

@andrerom I agree with the pool/item logic issue you brought up, but am focusing right now on the external API for this PR with the idea that we can continue making improvements to the internals even after this release.

@tedivm
Copy link
Member Author

tedivm commented Dec 16, 2015

At this point the number of changes has grown a bit, and I'm going to feel more comfortable having one more development release (v0.14) before committing the API to stone with a v1. Ideally the transition from v0.14 to v1.0 will not have any changes (basically I'm considering v0.14 to be a release candidate line for v1.0).

@tedivm tedivm changed the title [WIP] v1.0.0 [WIP] v1.0.0 / v0.14.0 Dec 16, 2015
@limenet
Copy link
Contributor

limenet commented Dec 16, 2015

Just to bring up a point about semantic versioning. I am aware of requirement 4:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

and also your installation recommendation.

Making 0.14.0 de facto equal to 1.0.0, however, will break things for people who defined a version like ~0.13 in their composer.json. And it is already possible (which is what I do) to list dev-v1.0.0-dev as version requirement.
Looking at the changelog I don't see a release so far which has as many breaking changes as v1.0.0 so it is possible a lot of people assume the library is "stable" (and ignore requirement 4 of semver).

What do you think about releasing it as 1.0.0-beta which makes it a pre-release (semver requirement 9 ff.)? Since a lot of people installed your package it might be worth considering.

Just my two cents, I hope you don't mind :)

@tedivm
Copy link
Member Author

tedivm commented Dec 16, 2015

I never mind feedback (if anything I'd prefer more of it honestly).

I probably wasn't clear but I don't expect v0.14.0 to just turn into v1.0.0 (although that would be nice). What I'm concerned about is the possibility that there will need to be a v0.14.1 and a v0.14.2. The number of changes that have gone into this new version here is pretty extreme, and the chances of bugs or even small api changes forcing changes is higher than I was expecting.

Part of me was considering v0.14 to be a transitional release (allowing for depreciated methods to reduce coding between versions), but the more I think about it the less that makes sense seeing as there are some BC breaking things already in this release which will force some coding.

@andrerom, what's your thought from the perspective of a downstream project with a lot of developer/users?

@limenet
Copy link
Contributor

limenet commented Dec 16, 2015

Glad to hear that.

Okay, I see, thanks for your clarification. I definitely see your point and in that case see the need for patch releases. To "solve" the issue with BC changes what about doing several betas for v1.0.0 (-beta1, -beta2, ...). This would agree with semver and with "standard-composer-user-intuition" since their 0.x-code wouldn't automatically updated.

Also, it would be really helpful if the docs (or at least changelog/upgrade notes) were already around once you push the code (as whichever release).

@tedivm
Copy link
Member Author

tedivm commented Dec 16, 2015

Documentation is being updated at tedious/www.stashphp.com#9. I need to do another round on it, and am also considering changing the theme of the documentation site (maybe migrating to jekyll).

@andrerom
Copy link
Contributor

From our point we only vary on patch releases, and are well aware you tend to do changes from one minor to the other in the pre 1.0 state.

So for us first a 0.14.x, then 1.0.0-beta is fine, just as fine if you jump directly on 1.0.0-beta if you feel the project is ready for that.

@Nyholm
Copy link

Nyholm commented Dec 16, 2015

As stated, doing 0.14.0 is fine. If you have written ~0.13 as a dependency, you are doing it wrong.

The difference between 0.14.0 and 1.0.0-beta is that 0.14.0 will be tested much more by the community.

Also, considering semver, isn't all 0.y.z considered as beta versions to 1.0.0?

tedivm added a commit that referenced this pull request Feb 1, 2016
@tedivm tedivm merged commit 569112b into master Feb 1, 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.

None yet

4 participants