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

Implement forward compatible Clock-Interface #1

Closed
wants to merge 3 commits into from

Conversation

heiglandreas
Copy link
Contributor

The stella-maris/clock package provides an interface based on the currently proposed status of PSR20. Due to the inactivity of the PSR20 working group this is a way to already provide interoperability while still maintaining forward compatibility. When the current status of PSR20 will be released at one point in time the stella-maris/clock package will extend the PSR20 interface so that this package becomes immeadiately PSR20 compatible without any further work necessary. In the long run the stella-maris/clock package will then be marked deprecated so that people can then use the PSR20 provided implementation.

Should the implementation of PSR20 change between now and a possible release then this interface will still exist and a possible implementation will need more code-changes anyhow so this interface will still provide some way of interoperability.

The implementation of the PSR20 polyfill has been moved to the stella-maris/clock interface which can be implemented independentyl from PSR20 and might allow some interoperability before the working group of the FIG manages to decide upon something.

The stella-maris/clock package provides an interface based on the
currently proposed status of PSR20. Due to the inactivity of the PSR20 working
group this is a way to already provide interoperability while still
maintaining forward compatibility. When the current status of PSR20 will
be released at one point in time the stella-maris/clock package will
extend the PSR20 interface so that this package becomes immeadiately
PSR20 compatible without any further work necessary. In the long run the
stella-maris/clock package will then be marked deprecated so that people
can then use the PSR20 provided implementation.

Should the implementation of PSR20 change between now and a possible
release then this interface will still exist and a possible
implementation will need more code-changes anyhow so this interface will
still provide some way of interoperability.

The implementation of the PSR20 polyfill has been moved to the
stella-maris/clock interface which can be implemented independentyl from
PSR20 and might allow some interoperability before the working group of
the FIG manages to decide upon something.
@heiglandreas
Copy link
Contributor Author

heiglandreas commented Apr 17, 2022

There are also PRs open for this on

to allow cross-library adoption.

Beware though, that I have removed the PSR20 Polyfill from the latest release of stella-maris/clock to provide a better forward compatibility.

As the PSR20 interface has not yet been finally agreed upon it might still change. The latest discussions around timezone-predicatbility came up with a changed interface where now should accept a timezone. Should that happen, then the polyfill would not be correct any more and everyone depending on the current polyfill would suddenly have a broken implementation.

Therefore basing the implementation on a separate interface that for now does not extend the PSR20 one but can fastly extend it makes more sense and makes the code future proof as - should the FIG decide upon the modified interface - this iplementation would still work and be interoperable with the current interfaces

@jeromegamez
Copy link
Member

Hey Andreas, and thank you for taking the time for this. I was already thinking about making kreait/clock wrap beste/clock since it is also based on the polyfilled PSR Clock interface.

I wouldn't mind making it based on your library, but could you help me understand the benefit of it, instead of using the PSR polyfill here?

@jeromegamez
Copy link
Member

jeromegamez commented Apr 17, 2022

As an added point, I already started using the PSR interface in the Firebase Admin SDK (kreait/firebase-php@f71584d) because I'm convinced this is the implementation we should aim for (I agree with everything you said in Discord about it) and would have to change it then there as well.

Honestly, if PSR-20 is not released in the form that I/we think it should be, I would perhaps consider not using it at all (and change the usage in my other projects to only rely on the beste/clock interface with bridges to other implementations like lcobucci/clock) for the time being 😕

@heiglandreas
Copy link
Contributor Author

heiglandreas commented Apr 17, 2022

The issue I see with the polyfill is that it provides an interface that is not yet finalized.

Polyfills are awesome to backport new functionality. For example to make the str_contains function also available for earlier PHP-versions. The advantage is, that the signature of the polyfill is defined and does not change any more (apart from complete BC breaks that might happen in future PHP-Versions, but let's for the moment skip that).

Here though you try to create a polyfill for something that is not yet finalized. Which means that it can still break.

When we are now using the polyfill with the signature public function now(): DateTimeImmutable; everything works out fine. Until the Working group decides to use public function now(DateTimeZone $timezone): DateTimeImmutable; as signature. And even if it is "only" public function now(DateTimeZone $tiemzone = null): DateTimeImmutable; - it is a different signature and as such the polyfill is not working any more.

As soon as the PSr\Clock\ClockInterface is actually available it will be different from the Polyfill and code will break.

So the advantage of using "my" package is that it is defined and already stable. So there is no chance of it to change. Therefore there is no polyfill needed.

And when the PSR20 will be released in it's current form it will be exactly the same as "my" interface. So I'll make "my" interface extend PSR20, remove all the content (as it's extending the already defined content) and nothing changes for the implementor of the interface, apart from the fact that their library is instantly PSR20 compatible.

When PSR20 will be released in a different form, then code that was created using "my" interface will still continue to work without any issues. And each and every library maintainer will then need to think about whether and how to implement the new PSR20 interface. So again: Nothing breaks.

The advantage that "my" Interface has over the PSR20 one: It is there, can publicly be used and it will make a switch to PSR20 as easy as possible.

And in the end the PSRs are just a way to agree upon a common interface. When 4 different maintainers of the 4 mostly used clock-implementations decide upon using one common interface, then that is already a standardization. No mater whether via a PSR or just any available interface.

I'm putting the my into quotes here as I don't see it as actually my interface. it is what the working group so far came up with and wanted to release and I'm just the one actually publishing a usable kind of "pre-release" release...

@jeromegamez
Copy link
Member

Sorry, it wasn't my intention to say something wrong by calling it "your" implementation. I'll merge the PRs and release new versions as soon as I get back at my computer, at latest after my vacation. Thanks for laying the reasoning out to me!

@heiglandreas
Copy link
Contributor Author

Sorry if that got across wrongly regarding the "my" 😂.

I was more refering to my own writing as I didn't find a better term than "my" implementation...

That had nothing to do with your comment!

And thanks for the feedback! Looking forward to the future times ahead 😁

@jeromegamez
Copy link
Member

Merged with 731f4c9, thank you! 🌺

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.

2 participants