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 some changes based on parent branch PR review #1

Open
wants to merge 3 commits into
base: universal-clock
Choose a base branch
from

Conversation

kevinquinnyo
Copy link

Implement some requested changes from the parent branch's PR. I went ahead and took a stab at this based on this your comment here @dpi: thephpleague#866 (comment)

This adds two new interfaces:

  • ClockInterface This is basically the spec from https://github.com/php-fig/fig-standards/blob/master/proposed/clock.md except without a return typehint to support PHP 5.6 for now
  • ClockAwareInterface - this may not be necessary but it seemed reasonable to have a getClock(): ClockInterface contract, given that we expect to be able to get a clock in both AccessToken and AbstractProvider.

This also removes some methods added in thephpleague#852, specifically:
- AccessToken::setTimeNow()
- AccessToken::getTimeNow()
- AccessToken::resetTimeNow()
- AccessToken::$timeNow

That technically represents a BC break, but:

  1. That code has not existed for very long and likely has very little usage.
  2. This should only break test code unless someone was doing something very strange in production.

For the existing AccessTokenInterface:
- remove the setClock() and getTimeNow()
- getTimeNow() is no longer needed anyway
- setClock() better not to have a setter on an interface in
this case
- both of these changes were BC breaks so they were removed

I believe the rest of the changes here are just cleaning up tests that used the older static method of setting the current time. The previous hack tearDownForBackwardsCompatibility() is no longer needed so it was removed.

This adds two new interfaces:
- `ClockInterface` This is basically the spec from
https://github.com/php-fig/fig-standards/blob/master/proposed/clock.md
except without a return typehint to support PHP 5.6 for now
- `ClockAwareInterface` - this may not be necessary but it seemed
reasonable to have a `getClock(): ClockInterface` contract, given that
we expect to be able to get a clock in both `AccessToken` and
`AbstractProvider`.

This also removes some methods added in
thephpleague#852, specifically:
    - `AccessToken::setTimeNow()`
    - `AccessToken::getTimeNow()`
    - `AccessToken::resetTimeNow()`
    - `AccessToken::$timeNow`

That technically represents a BC break, but:
1. That code has not existed for very long and likely has very little
usage.
2. This *should* only break test code unless someone was doing something
very strange in production.

For the existing `AccessTokenInterface`:
    - remove the `setClock()` and `getTimeNow()`
        - `getTimeNow()` is no longer needed anyway
        - `setClock()` better not to have a setter on an interface in
        this case
        - both of these changes were BC breaks so they were removed

I believe the rest of the changes here are just cleaning up tests that
used the older static method of setting the current time. The previous
hack `tearDownForBackwardsCompatibility()` is no longer needed so it was
removed.
@kevinquinnyo kevinquinnyo changed the title Implement some changed based on parent branch PR review Implement some changes based on parent branch PR review Jul 18, 2021
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.

1 participant