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

[5.3] Remove broken timestamps option #12617

Merged
merged 1 commit into from Mar 4, 2016

Conversation

Projects
None yet
5 participants
@acasar
Contributor

acasar commented Mar 3, 2016

Following #12572 I think it's best to do it in 5.3 then.

@taylorotwell @vlakoff

@vlakoff

This comment has been minimized.

Show comment
Hide comment
@vlakoff

vlakoff Mar 3, 2016

Contributor

The feature doesn't work so I think it's not a problem to remove it in 5.2.

And if someone was extending performUpdate or performInsert protected methods, as the parameter was optional this shouldn't be breaking under normal circumstances.

Contributor

vlakoff commented Mar 3, 2016

The feature doesn't work so I think it's not a problem to remove it in 5.2.

And if someone was extending performUpdate or performInsert protected methods, as the parameter was optional this shouldn't be breaking under normal circumstances.

@GrahamCampbell

This comment has been minimized.

Show comment
Hide comment
@GrahamCampbell

GrahamCampbell Mar 3, 2016

Member

5.3 is fine imo.

Member

GrahamCampbell commented Mar 3, 2016

5.3 is fine imo.

@acasar

This comment has been minimized.

Show comment
Hide comment
@acasar

acasar Mar 3, 2016

Contributor

@vlakoff We can still remove the feature from 5.1 and 5.2 without modifying method parameters, but I'm not sure if it's worth it.

Contributor

acasar commented Mar 3, 2016

@vlakoff We can still remove the feature from 5.1 and 5.2 without modifying method parameters, but I'm not sure if it's worth it.

@acasar

This comment has been minimized.

Show comment
Hide comment
@acasar

acasar Mar 3, 2016

Contributor

No idea why tests are failing :/ But it seems unrelated to this PR.

Contributor

acasar commented Mar 3, 2016

No idea why tests are failing :/ But it seems unrelated to this PR.

@GrahamCampbell

This comment has been minimized.

Show comment
Hide comment
@GrahamCampbell

GrahamCampbell Mar 3, 2016

Member

Try rebasing your fork.

Member

GrahamCampbell commented Mar 3, 2016

Try rebasing your fork.

@mark86092

This comment has been minimized.

Show comment
Hide comment
@mark86092

mark86092 Mar 4, 2016

Contributor

The build fail due to the change of symfony class, I send a PR #12623.

Contributor

mark86092 commented Mar 4, 2016

The build fail due to the change of symfony class, I send a PR #12623.

taylorotwell added a commit that referenced this pull request Mar 4, 2016

Merge pull request #12617 from acasar/remove-timestamps
[5.3] Remove broken timestamps option

@taylorotwell taylorotwell merged commit 02923ca into laravel:master Mar 4, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
StyleCI The StyleCI analysis has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment