Bug 6684 - koha-remove should check the number of arguments it gets
Summary: koha-remove should check the number of arguments it gets
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Robin Sheat
QA Contact: Bugs List
URL:
Keywords:
Depends on: 7924
Blocks: 6416 4880
  Show dependency treegraph
 
Reported: 2011-08-09 10:31 UTC by Magnus Enger
Modified: 2013-12-05 19:59 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
A patch to the koha-remove script (3.24 KB, patch)
2011-11-16 17:48 UTC, Simon Story
Details | Diff | Splinter Review
Patch to bugs 6684 6997 4880 (3.41 KB, patch)
2011-11-18 15:01 UTC, Simon Story
Details | Diff | Splinter Review
Patches to bugs 6684 6997 4880 6684 koha-remove should check the number of arguments it gets 6997 koha-remove leaves system in inconsistent state if there is an error 4880 koha-remove sometimes fails because user is logged intkoha-remove sometimes fails b (3.21 KB, patch)
2012-04-07 10:51 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6684: Fixes to koha-remove (3.32 KB, patch)
2012-05-13 11:00 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2011-08-09 10:31:56 UTC
Currently, koha-remove does not check that at least one instancename was provided as an argument, so running it with arguments causes an Apache reload, which might come as a surprise. 

If no instancename is given as argument it should probably print it's usage.
Comment 1 Simon Story 2011-11-16 17:48:42 UTC Comment hidden (obsolete)
Comment 2 Simon Story 2011-11-18 15:01:40 UTC Comment hidden (obsolete)
Comment 3 Magnus Enger 2012-03-27 19:43:03 UTC
This should be "Needs signoff", I think?
Comment 4 Chris Cormack 2012-04-07 10:51:06 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2012-04-07 10:52:29 UTC
Hi Simon, sorry about the long delay on this. 

The patch applied with a few conflicts, not sure I tidied them up all correctly can you take a look at it and check it looks ok please.
Comment 6 Jared Camins-Esakov 2012-05-13 11:00:02 UTC
Created attachment 9533 [details] [review]
Bug 6684: Fixes to koha-remove

Fixes bugs 6684 6997 4880:
6684 koha-remove should check the number of arguments it gets
6997 koha-remove leaves system in inconsistent state if there is an error
4880 koha-remove sometimes fails because user is logged in

Amended by chrisc@catalyst.net.nz
Trying to resolve conflicts.

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Rebased on top of bug 7924.
Comment 7 Jared Camins-Esakov 2012-05-13 11:00:46 UTC
The signed off patch will not apply without 7924.
Comment 8 Paul Poulain 2012-05-28 09:30:39 UTC
QA comment : tiny patch, nothing specific to say about QA
Comment 9 Paul Poulain 2012-05-28 09:31:01 UTC
Patch pushed,
Welcome Simon Story, you're the 182th committer !
Comment 10 Robin Sheat 2012-05-28 12:33:18 UTC
Only just really noticed this now, but I have to say that I really appreciated the commenting added.
Comment 11 Chris Cormack 2012-05-28 12:34:33 UTC
Will be in 3.8.2