Skip to content

Allow dynamic intervals in DATE_ADD for SQLite#2984

Closed
fogs wants to merge 1 commit intodoctrine:masterfrom
fogs:master
Closed

Allow dynamic intervals in DATE_ADD for SQLite#2984
fogs wants to merge 1 commit intodoctrine:masterfrom
fogs:master

Conversation

@fogs
Copy link
Copy Markdown
Contributor

@fogs fogs commented Jan 21, 2018

Fixing #2983

Copy link
Copy Markdown
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Requires test additions

@fogs
Copy link
Copy Markdown
Contributor Author

fogs commented Jan 21, 2018

Can you give an example of additional tests required? I did not find tests for DATE_ADD at all.

@Ocramius
Copy link
Copy Markdown
Member

#2482 includes similar changes

@fogs
Copy link
Copy Markdown
Contributor Author

fogs commented Feb 10, 2018

@Ocramius,
What can I do to get this change merged? What type of tests would you like to see? Can you point me in the direction of already existing tests that I can reuse? I did not find anything but I have never contributed to a big project like this.

Thanks!

@Ocramius
Copy link
Copy Markdown
Member

It doesn't matter if it's a big or small project: if an addition or change isn't backed by tests, it should not land in the codebase. This should be something that applies also for your own projects ;-)

As I've linked above, #2482 has similar tests - see https://github.com/vhraban/dbal/blob/d81e2a4e5937e045df4d79c8cad073d306c4abd6/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php

@fogs
Copy link
Copy Markdown
Contributor Author

fogs commented Feb 11, 2018

Closing in favor of #3019 which comes with the requested tests

@fogs fogs closed this Feb 11, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants