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

Modify meta doc #1230

Merged
merged 23 commits into from
Jul 13, 2021
Merged

Modify meta doc #1230

merged 23 commits into from
Jul 13, 2021

Conversation

heiglandreas
Copy link
Contributor

This PR contains some changes that should make it more clear why certain decissions were taken the way we took them.

This should make some of the decisions taken more clear to people using
the interface
The UTCClock is not really needed for a minimum implementation
This method is not really needed on a FrozenClock. And especially as a
FrozenClock should be, well, frozen, there should be no way of modifying
that information
@heiglandreas heiglandreas requested a review from a team as a code owner April 14, 2021 07:54
Scheduler tasks are not part of retrieving the time and should be either
handled by the implementation or by a different PSR
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
@cseufert
Copy link
Contributor

Looks great thanks @heiglandreas

@heiglandreas
Copy link
Contributor Author

Anything I can do to go on with this PSR?

Copy link

@kylekatarnls kylekatarnls left a comment

Choose a reason for hiding this comment

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

Few typos sound to have truncated word/sentence

Thanks to @kylekatarnis for spotting them!
@heiglandreas heiglandreas requested a review from shadowhand June 1, 2021 05:24
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
Comment on lines +55 to +56
(This list is not exhaustive!)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(This list is not exhaustive!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though that should not be necessary I'd rather keep it to make clear that there might be other packages that provide the same value. Yes, we introduced that list with "some popular packages … are". But there will be people not reading that. And the oneliner right underneath the table is catching the eye better than the message in the copy-text above.

proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
proposed/clock-meta.md Outdated Show resolved Hide resolved
Copy link
Contributor

@shadowhand shadowhand left a comment

Choose a reason for hiding this comment

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

One remaining suggestion, but otherwise looks great!

proposed/clock-meta.md Outdated Show resolved Hide resolved
@BusterNeece BusterNeece merged commit b3b3652 into php-fig:master Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants