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

Add support for DateTimeImmutable objects #82

Merged
merged 2 commits into from
Aug 14, 2019
Merged

Add support for DateTimeImmutable objects #82

merged 2 commits into from
Aug 14, 2019

Conversation

geerteltink
Copy link
Contributor

This PR adds support for DateTimeImmutable objects by using the DateTimeInterface where needed.

@@ -1,2 +1,3 @@
composer.lock

Choose a reason for hiding this comment

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

The composer.lock is ignored and I guess we don't need this repository to cache dependencies version.

And I think the composer.lock is also remove from Git version control.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding the composer.lock file to version control is useful if you have a lot of dependencies so you can track down which dependencies might have broken and not respecting semver. Since time-ago doesn't rely on any dependency (except dev dependencies) it's pretty much useless to include it.

Choose a reason for hiding this comment

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

Ok. I agree with you.

Copy link

@peter279k peter279k left a comment

Choose a reason for hiding this comment

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

To let the DateTime and DateTimeImmutable classes be inherited the DateTimeInterface.

Using the DateTimeInterface to be for the type hint and comment annotations is fine.

@andersahn
Copy link
Collaborator

This is a very good change 👍

@andersahn andersahn requested a review from jimmiw August 10, 2019 16:17
@jimmiw jimmiw merged commit 2fd774a into jimmiw:master Aug 14, 2019
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.

4 participants