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

Remove non-prepared variant of database select wrappers #3553

Merged
merged 2 commits into from
Apr 9, 2018

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Mar 19, 2018

The non-prepared variant of the database select wrappers have been
deprecated for a long time, but left around because of the need of
projects to update any instruments they might have that uses them.

This finally removes the code, now that users have had time to
update any calls to them and all references in the LORIS codebase
have been removed.

See also: comments on #3544

@driusan driusan added Release: Add to release notes PR whose changes should be highlighted in the release notes Cleanup PR or issue introducing/requiring at least one clean-up operation Release: Breaking changes PR that contains changes that might impact the code or accepted practices of active projects [branch] major labels Mar 19, 2018
@johnsaigle
Copy link
Contributor

This is a very good idea Dave! Happy that it's happening.

@ridz1208 ridz1208 added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Mar 19, 2018
@ridz1208
Copy link
Collaborator

@driusan 35 seconds later... you have a conflict

The non-prepared variant of the database select wrappers have been
deprecated for a long time, but left around because of the need of
projects to update any instruments they might have that uses them.

This finally removes the code, now that users have had time to
update any calls to them and all references in the LORIS codebase
have been removed.
@driusan driusan force-pushed the RemoveDeprecatedDatabaseFunctions branch from 14d971c to 33ae1ab Compare March 19, 2018 18:05
* @access public
* @deprecated Use pselect wrappers instead
*/
function selectRow($query, &$result)
Copy link
Contributor

Choose a reason for hiding this comment

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

docs/instruments/NDB_BVL_Instrument_mri_parameter_form.class.inc will be sad

Copy link
Collaborator

Choose a reason for hiding this comment

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

resolved

* @access public
* @deprecated Use pselect wrappers instead
*/
function select($query, &$result)
Copy link
Contributor

Choose a reason for hiding this comment

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

poor tools/excelDump.php

Copy link
Collaborator

Choose a reason for hiding this comment

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

resolved

@driusan driusan removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Mar 19, 2018
@ridz1208
Copy link
Collaborator

@xlecours Done and Done please re-review

@kongtiaowang kongtiaowang added the Passed manual tests PR has been successfully tested by at least one peer label Mar 26, 2018
Copy link
Contributor

@PapillonMcGill PapillonMcGill left a comment

Choose a reason for hiding this comment

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

haven't tried it and don't know of project using it. Did a grep on these function name and returned no usage.

@driusan driusan merged commit 64d3093 into aces:major Apr 9, 2018
@ridz1208 ridz1208 added this to the 20.0 milestone Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup PR or issue introducing/requiring at least one clean-up operation Passed manual tests PR has been successfully tested by at least one peer Release: Add to release notes PR whose changes should be highlighted in the release notes Release: Breaking changes PR that contains changes that might impact the code or accepted practices of active projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants