> On July 23, 2014, 12:05 p.m., Santhosh Edukulla wrote: > > tools/marvin/marvin/marvinInit.py, line 114 > > <https://reviews.apache.org/r/23847/diff/1/?file=640532#file640532line114> > > > > please use is not None, rather != > > Miguel Ferreira wrote: > I will do this this time, but if something is broken, I fix it and you > see better ways of fixing it, please be my guest and do it yourself. after > all that is the spirit of open-source, right? > > Santhosh Edukulla wrote: > sure, but lets make the submitter realize some better ways if there are, > after all it helps the review submission and cleaner approach Agree? The > review iam doing signifies its open source for your submission. Functionally, > modifying your review submission is like altering the behavior and may add > bugs, also resubmitting my review again. > > Miguel Ferreira wrote: > Patch was not in. Will submit it again.
Of course that the reviewer has the responsibility to make sure that the submitter adheres to the guidelines of a project. But the reviewer is not the manager of the submitter. I could even understand if this patch (and others I've submitted) where about new features. But they are not. They are fixing things that are broken. If I decide to take the time to contribute back what I do to fix the code in the project, and address bugs in the system, I do not expect to get push back based on "there are better ways to do it". Because it was broken to begin with, so my patch is in fact a better way to do it. I'll be very happy if you say, sorry but I don't accept your patch because I know better ways to fix the code and I will do it myself. However, what you are doing is leveraging my time to get things done the way you see best, and again, I'm not here to be managed. - Miguel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23847/#review48496 ----------------------------------------------------------- On July 23, 2014, 12:17 p.m., Miguel Ferreira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23847/ > ----------------------------------------------------------- > > (Updated July 23, 2014, 12:17 p.m.) > > > Review request for cloudstack, daan Hoogland, Santhosh Edukulla, and Hugo > Trippaers. > > > Repository: cloudstack-git > > > Description > ------- > > Running a marvin test that does not specify a zone in the config file will > produce an exception while parsing the config: > > Exception Occurred Under init ['Traceback (most recent call last):\n', ' > File "/usr/local/lib/python2.7/site-packages/marvin/marvinInit.py", line 118, > in __setHypervisorAndZoneInfo\n for zone in self.__parsedConfig.zones:\n', > "TypeError: 'NoneType' object is not iterable\n"] > > This happens when the code starts iterating over a list of zones that does > not exist. > I've added a check for the existence of the list of zones before iterating. > > > Diffs > ----- > > tools/marvin/marvin/marvinInit.py ce9b43f > > Diff: https://reviews.apache.org/r/23847/diff/ > > > Testing > ------- > > Tests now run without the mentioned exception > > > Thanks, > > Miguel Ferreira > >
