Description
Tomás Cohen Arazi (tcohen)
2013-03-14 18:35:41 UTC
When stopwords related code is removed this should include the database table stopwords. Created attachment 17341 [details] [review] Bug 9819 - 'stopwords'-related code removed This patch removes code related to stopwords usage. The following methods are removed: C4::Search->remove_stopwords C4::Context->stopwords C4::Context->_new_stopwords And the buildQuery API was changed (removed the \@removed_stopwords return value). A follow-up is provided for database changes, to make rebasing easier. To test: - Apply this patch - Do some searches in both intranet and opac interfaces - Nothing should break Sponsored-by: Universidad Nacional de Córdoba Created attachment 17342 [details] [review] Bug 9819 - Remove 'stopwords' table from kohastructure.sql Just that. To test: - Apply the patch - Do a fresh install - The database structure should be created flawlessly - There should not be a 'stopwords' table created Sponsored-by: Universidad Nacional de Córdoba Created attachment 17343 [details] [review] Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table To test: - Apply the patch - Will be prompted to run the updater - The 'stopwords' database should be removed ( test on a mysql prompt with > SHOW TABLES LIKE 'stopwords'; and should get no results) Sponsored-by: Universidad Nacional de Córdoba Hi Tomas, Could you also explain on this bug report why the stopwords should be removed? The use of stopwords was limited to NoZebra - so this bug is a consequence of removing all NoZebra related code. Created attachment 17558 [details] [review] Bug 9819 - 'stopwords'-related code removed This patch removes code related to stopwords usage. The following methods are removed: C4::Search->remove_stopwords C4::Context->stopwords C4::Context->_new_stopwords And the buildQuery API was changed (removed the \@removed_stopwords return value). A follow-up is provided for database changes, to make rebasing easier. To test: - Apply this patch - Do some searches in both intranet and opac interfaces - Nothing should break Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 17559 [details] [review] Bug 9819 - Remove 'stopwords' table from kohastructure.sql Just that. To test: - Apply the patch - Do a fresh install - The database structure should be created flawlessly - There should not be a 'stopwords' table created Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 17560 [details] [review] Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table To test: - Apply the patch - Will be prompted to run the updater - The 'stopwords' database should be removed ( test on a mysql prompt with > SHOW TABLES LIKE 'stopwords'; and should get no results) Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> QA comment: Great job Tomás. All looks good. But there are 2 smalls things 1/ the structure file for PostgreSQL contains the "create table" statement for the stopwords table. I know this file is not updated, but while you're at it you could delete it. 2/ The stopwords sql file for spanish has been introduced into master before you provided your first patch. It should be deleted. Marked as Failed QA for 2. Created attachment 17587 [details] [review] Bug 9819 - QA Followup: Stopwords vestiges in Pg and es-ES removed Gotcha. To+ Tomás, thank you for your followup! I found another issue: some unit tests in t/db_dependent/Search.t fail: # Failed test 'getRecords on _ALLRECORDS PQF returned all records' # at t/db_dependent/Search.t line 387. # got: undef # expected: '178' # Failed test 'getRecords PQF author search for Lessig returned proper number of matches' # at t/db_dependent/Search.t line 393. # got: undef # expected: '4' # Failed test 'getRecords CCL author search for Lessig returned proper number of matches' # at t/db_dependent/Search.t line 399. # got: undef # expected: '4' # Failed test 'getRecords CQL author search for Lessig returned proper number of matches' # at t/db_dependent/Search.t line 405. # got: undef # expected: '4' # Looks like you failed 4 tests of 75. They passed with success before yours patches. (In reply to comment #12) > They passed with success before yours patches. What data set can I use to test it in my box? (In reply to comment #13) > (In reply to comment #12) > > They passed with success before yours patches. > > What data set can I use to test it in my box? No idea! On master, the prove t/db_dependent/Search.t command does not return 0? It looks like we got stuck on this... but would be a nice clean-up? :) Some of the code seems to have been removed since - patches don't apply. *** Bug 11110 has been marked as a duplicate of this bug. *** I'd also like to see this make it's way in to Koha Created attachment 45685 [details] [review] Bug 9819 - 'stopwords'-related code removed This patch removes code related to stopwords usage. The following methods are removed: C4::Search->remove_stopwords C4::Context->stopwords C4::Context->_new_stopwords And the buildQuery API was changed (removed the \@removed_stopwords return value). A follow-up is provided for database changes, to make rebasing easier. To test: - Apply this patch - Do some searches in both intranet and opac interfaces - Nothing should break Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 45686 [details] [review] Bug 9819 - Remove 'stopwords' table from kohastructure.sql Just that. To test: - Apply the patch - Do a fresh install - The database structure should be created flawlessly - There should not be a 'stopwords' table created Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 45687 [details] [review] Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table To test: - Apply the patch - Will be prompted to run the updater - The 'stopwords' database should be removed ( test on a mysql prompt with > SHOW TABLES LIKE 'stopwords'; and should get no results) Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 45688 [details] [review] Bug 9819 - QA Followup: Stopwords vestiges in es-ES removed Gotcha. To+ Created attachment 45689 [details] [review] Bug 9819: Make the tests pass Patches rebased against master, please test. Created attachment 45791 [details] [review] Bug 9819 - 'stopwords'-related code removed This patch removes code related to stopwords usage. The following methods are removed: C4::Search->remove_stopwords C4::Context->stopwords C4::Context->_new_stopwords And the buildQuery API was changed (removed the \@removed_stopwords return value). A follow-up is provided for database changes, to make rebasing easier. To test: - Apply this patch - Do some searches in both intranet and opac interfaces - Nothing should break Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45792 [details] [review] Bug 9819 - Remove 'stopwords' table from kohastructure.sql Just that. To test: - Apply the patch - Do a fresh install - The database structure should be created flawlessly - There should not be a 'stopwords' table created Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45793 [details] [review] Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table To test: - Apply the patch - Will be prompted to run the updater - The 'stopwords' database should be removed ( test on a mysql prompt with > SHOW TABLES LIKE 'stopwords'; and should get no results) Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45794 [details] [review] Bug 9819 - QA Followup: Stopwords vestiges in es-ES removed Gotcha. To+ Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45795 [details] [review] Bug 9819: Make the tests pass Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45854 [details] [review] Bug 9819 - 'stopwords'-related code removed This patch removes code related to stopwords usage. The following methods are removed: C4::Search->remove_stopwords C4::Context->stopwords C4::Context->_new_stopwords And the buildQuery API was changed (removed the \@removed_stopwords return value). A follow-up is provided for database changes, to make rebasing easier. To test: - Apply this patch - Do some searches in both intranet and opac interfaces - Nothing should break Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45855 [details] [review] Bug 9819 - Remove 'stopwords' table from kohastructure.sql Just that. To test: - Apply the patch - Do a fresh install - The database structure should be created flawlessly - There should not be a 'stopwords' table created Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45856 [details] [review] Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table To test: - Apply the patch - Will be prompted to run the updater - The 'stopwords' database should be removed ( test on a mysql prompt with > SHOW TABLES LIKE 'stopwords'; and should get no results) Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 45857 [details] [review] Bug 9819 - QA Followup: Stopwords vestiges in es-ES removed Gotcha. To+ Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 45858 [details] [review] Bug 9819: Make the tests pass Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 46092 [details] [review] Bug 9819 [QA Followup] - Removed missed stopwords refs, remove Schema file Pushed to master, thanks Tomas and Jonathan! |