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

Fix message storage of recorded at for mysql 8.0 #33

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Robertbaelde
Copy link
Contributor

This small change in the LaravelMessageRepository will fix the issue with mysql8 datetime:

Issue in LaravelEventSauce #25
"parent" issue in EventSauce: EventSaucePHP/EventSauce#108

I'm aware of the MySQL8DateFormatting but I think formatting the date in the right way for the storage should be a concern of the repository.

Besides that, I think this would improve the onboarding experience of new users of this package.

@Robertbaelde
Copy link
Contributor Author

@frankdejonge @rahmanii0 Curious about the vision you both have in regards to the versioning of this package. Since EventSauce .8 to 1.2 introduced some breaking changes, I think it would be easier for maintenance of this package to follow EventSauce's major version bump.

If we don't do this, we'll end up with If checks through this package in order to support all versions.
Happy with both ways, but think it's a nice time to discuss since this PR encountered the issue.

@Robertbaelde Robertbaelde changed the title Fix message storage using correct recorded_at Fix message storage of recorded at for mysql 8.0 Jan 20, 2022
@frankdejonge
Copy link
Member

@Robertbaelde I think it makes sense to drop 0.8 and bump the major version up.

@simensen
Copy link

simensen commented Apr 8, 2022

Do we still need to do this if we replace LaravelMessageRepository with the core Illuminate message repository? #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants