User tests: Successful: Unsuccessful:
wrong encoding for bytea
wrong inner join update syntax
on postgresql
enable smart search plugin
edit & save a tag
tag is saved
works as before on mysql
multiple errors
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_finder |
well yes, i could, but atm with this silly PR i just want to give to the few postgresql users the opportunity to play a little bit with j4, and help us to discover more bugs.....prepared statement conversion should be matter for another pr imho
At the moment only you and me did prepared statement conversion it should be a strict requirement on all pr touching a sql statement to convert it to use prepared statements.
There was a PLT motion about this topic:
If we don't required this we will still get new non prepared queries in patches.
i'd be more than happy if this motion have been taken in serious count before merging all & more,
but honestly this has not been happened....
At the moment only you and me did prepared statement conversion it should be a strict requirement on all pr touching a sql statement to convert it to use prepared statements.
Forcing unrelated changes into pull requests is exactly how you end up making pull requests more complex to review and more error prone. Converting existing queries should be pull requests of their own, writing new queries it is fine to mandate this. "You touched it so you need to change it" isn't a good workflow. Same goes with the random unrelated code style fixes that are commonly requested.
Forcing unrelated changes into pull requests is exactly how you end up making pull requests more complex to review and more error prone. Converting existing queries should be pull requests of their own, writing new queries it is fine to mandate this. "You touched it so you need to change it" isn't a good workflow. Same goes with the random unrelated code style fixes that are commonly requested.
This is partly true, but whats the better way? If no queries get updated to PS only if we find more then 2 people to do this job we will never finish this task.
Anyway I don't request to update all queries in this file only the one that get touched and tested afterwards. In a perfect would we would have tests at the same time, or the PR includes a test if non exists but as we all know tests are still missing all over the place.
It's ok for me if at least new queries use PS...
@alikon Will you work on this, make Harald happy with just changing those queries to PS which you have modified, and handle Hannes‘ comments? If so, ping me when ready for tests. But if you have little time: Shall I help with picking up that work and making PR for your repo so you just can merge with 1 click? If so, ping me and I will do later today.
Labels |
Added:
?
|
I have tested this item
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-09-17 08:47:35 |
Closed_By | ⇒ | wilsonge |
Thanks!
All new pr should use prepared statements, could you update this pr?