Bug #107752 for Map-Tube: Inconsistency in checking of arguments

Tue Oct 13 10:37:18 2015 SKIM [...] cpan.org - Ticket created

Subject:

Inconsistency in checking of arguments

Hi, There are some inconsistency in checking of arguments. get_stations(): - Checks missing argument. - Checks invalid argument. get_node_by_id(): - Checks missing argument. - Don't check invalid argument. get_node_by_name(): - Don't check missing argument. - Don't check invalid argument. get_line_by_id(): - Don't check missing argument. - Don't check invalid argument. get_line_by_name(): - Don't check missing argument. - Don't check invalid argument. Regards, Michal

Tue Oct 13 11:10:35 2015 MANWAR [...] cpan.org - Severity Important added

Tue Oct 13 11:10:46 2015 MANWAR [...] cpan.org - Taken

Tue Oct 13 11:12:07 2015 MANWAR [...] cpan.org - Correspondence added

Hi, Thanks for reporting the inconsistency. I will try to get that fixed tomorrow. Many Thanks. Best Regards, Mohammad S Anwar

Tue Oct 13 11:12:07 2015 The RT System itself - Status changed from 'new' to 'open'

Wed Oct 14 05:31:03 2015 MANWAR [...] cpan.org - Broken in 3.03 added

Wed Oct 14 05:33:33 2015 MANWAR [...] cpan.org - Correspondence added

Hi, I have patched the code in Map::Tube v3.04 as below: https://github.com/Manwar/Map-Tube https://metacpan.org/release/MANWAR/Map-Tube-3.04 Best Regards, Mohammad S Anwar

Wed Oct 14 05:33:49 2015 MANWAR [...] cpan.org - Status changed from 'open' to 'patched'

Wed Oct 14 06:58:40 2015 SKIM [...] cpan.org - Correspondence added

Dne St 14.říj.2015 05:33:33, MANWAR napsal(a): Hi, Show quoted text

> I have patched the code in Map::Tube v3.04 as below:

Thanks. 1) Missing Map::Tube::Exception::InvalidLineId 2) This changes errors in get_shortest_route(). Instead "ERROR: Received invalid FROM node 'Foo'" this goes "ERROR: Invalid station name." M.

Wed Oct 14 09:14:15 2015 MANWAR [...] cpan.org - Correspondence added

Hi, Fixed missing exception error in Map::Tube v3.05, thanks for reporting. Much appreciated. I will look into the other issue when I get time. Best Regards, Mohammad S Anwar

Wed Oct 14 16:09:05 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-10-14 09:14:15, MANWAR wrote: Show quoted text

> Hi, > > Fixed missing exception error in Map::Tube v3.05, thanks for > reporting. Much appreciated. > > I will look into the other issue when I get time.

Currently every Map::Tube::* modules seems to have broken tests. I just uploaded Map::Tube::Berlin 0.10 which accepts the old and new error messages in the test suite.

Wed Oct 14 16:09:27 2015 SREZIC [...] cpan.org - Requestor SREZIC added

Thu Oct 15 05:28:45 2015 MANWAR [...] cpan.org - Requestor GWS added

Thu Oct 15 05:42:05 2015 MANWAR [...] cpan.org - Correspondence added

Hi Slaven, I noticed that when Michal raised the issue first. I am embarrassed to be honest by this. I have now added Gisbert as well in this conversation as he also has stake into this project. I have made error message consistent throughout as requested by Michal earlier but that might break some/all tests for you guys. Please accept my apology in advance. This made me thinking how best to deal with such kind of changes without affecting so much users. I am open to any suggestions. I have one suggestion though that I would like to put forward and happy to receive your comment on the same. "Collect all unit tests from every Map::Tube::* distributions and put it in one place, ideally in a new method ok_map_functions() in the package Test::Map::Tube, similar to an existing method ok_map(). This way, anyone creating new map with Map::Tube, do not have to worry about testing the functionalities. Also depending on user requirements, they can either test just the map by calling ok_map() or just the map functions by calling ok_map_functions() or both." My objective is to keep the distribution using Map::Tube as light as possible with as few dependencies as possible. This would encourage more people to add more map ideally :-) Many Thanks. Best Regards, Mohammad S Anwar

Thu Oct 15 06:07:54 2015 SKIM [...] cpan.org - Correspondence added

Dne Čt 15.říj.2015 05:42:05, MANWAR napsal(a): Hi Mohammad, I think, that you can create same error as "ERROR: Received invalid FROM node 'Foo'" in get_shortest_route() method for: 1) Backward compatibility. 2) More readable error in this routine. When method returns "Invalid station name", you don't know, which station is it. Regards, M.

Thu Oct 15 07:05:00 2015 MANWAR [...] cpan.org - Correspondence added

Hi Michal, Please review the changes just pushed, this should bring in some kind of stability, hopefully. https://github.com/Manwar/Map-Tube https://metacpan.org/release/MANWAR/Map-Tube-3.06 Many Thanks. Best Regards, Mohammad S Anwar

Thu Oct 15 08:04:19 2015 SKIM [...] cpan.org - Correspondence added

Dne Čt 15.říj.2015 07:05:00, MANWAR napsal(a): Hi, Show quoted text

> Please review the changes just pushed, this should bring in some kind > of stability, hopefully. > > https://github.com/Manwar/Map-Tube > https://metacpan.org/release/MANWAR/Map-Tube-3.06

It's functional. I release Map::Tube::Bucharest 0.07 with these tests. So, changed API force me to fix all packages :-p Regards, M.

Thu Oct 15 09:19:10 2015 GWS [...] cpan.org - Correspondence added

Thanks for forwarding this to me, Mohammad! Am 2015-10-15 05:42:05, MANWAR schrieb: Show quoted text

> Hi Slaven, >

Show quoted text

> > "Collect all unit tests from every Map::Tube::* distributions and put > it in one place, ideally in a new method ok_map_functions() in the > package Test::Map::Tube, similar to an existing method ok_map(). This > way, anyone creating new map with Map::Tube, do not have to worry > about testing the functionalities. Also depending on user > requirements, they can either test just the map by calling ok_map() or > just the map functions by calling ok_map_functions() or both." > > My objective is to keep the distribution using Map::Tube as light as > possible with as few dependencies as possible. This would encourage > more people to add more map ideally :-)

Sounds good to me! \Gisbert

Thu Oct 15 16:38:35 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-10-15 05:42:05, MANWAR wrote: Show quoted text

> Hi Slaven, > > I noticed that when Michal raised the issue first. I am embarrassed to > be honest by this. > I have now added Gisbert as well in this conversation as he also has > stake into this project. > > I have made error message consistent throughout as requested by Michal > earlier but that might break some/all tests for you guys. Please > accept my apology in advance. > > This made me thinking how best to deal with such kind of changes > without affecting so much users. I am open to any suggestions. I have > one suggestion though that I would like to put forward and happy to > receive your comment on the same. > > "Collect all unit tests from every Map::Tube::* distributions and put > it in one place, ideally in a new method ok_map_functions() in the > package Test::Map::Tube, similar to an existing method ok_map(). This > way, anyone creating new map with Map::Tube, do not have to worry > about testing the functionalities. Also depending on user > requirements, they can either test just the map by calling ok_map() or > just the map functions by calling ok_map_functions() or both." > > My objective is to keep the distribution using Map::Tube as light as > possible with as few dependencies as possible. This would encourage > more people to add more map ideally :-)

The good thing is that it's just us four (I think) having Map::Tube::* modules, and there is probably no "GreyPAN" code directly depending on Map::Tube, so backward incompatible changes are relatively easy to manage. So if this does not happen too often then I am fine with the situation as it is now. I just have a second life as a CPAN Tester, and I review all fail reports my smokers generate, so yesterday I had a lot of reports to look over (about 500 fail reports for Map::Tube::* modules)... Regards, Slaven

Fri Oct 16 05:46:32 2015 MANWAR [...] cpan.org - Correspondence added

Hi Slaven, I understand the pain you have been through. I will do my best not to repeat this again. Regarding my proposed solution to test the basic functions provided by Map::Tube, I have added new function ok_map_functions() in the package Test::Map::Tube v0.09. Please give your comments/suggestions when you have time. https://github.com/Manwar/Test-Map-Tube https://metacpan.org/release/MANWAR/Test-Map-Tube-0.09 Many Thanks. Best Regards, Mohammad S Anwar

Fri Oct 16 05:46:34 2015 MANWAR [...] cpan.org - Fixed in 3.06 added

Sun Oct 18 07:28:46 2015 MANWAR [...] cpan.org - Correspondence added

Closing it down. Thanks all for your feedbacks. Best Regards, Mohammad S Anwar

Sun Oct 18 07:28:48 2015 MANWAR [...] cpan.org - Status changed from 'patched' to 'resolved'