Hi David, Thank you for your reply. I will look into your suggestions and modify it appropriately.
On 2/9/11, David Kalnischkies <[email protected]> wrote: > Hi Ishan, > > On Tue, Feb 8, 2011 at 16:46, Ishan Jayawardena <[email protected]> wrote: >> I tried to come up with a fix. > > First of all, the patch really fixes the issue for rdepends, so you are on > the right track, but… > >> Please find the attached patch and give >> me your feedback. > > * the indent-style is wrong. I know that the indent-style is… lets say > strange, but thats how it is and changing it would just destroy the > history > in the vcs. The style is easiest described by indent with 3 spaces on each > curly bracket - but replace 8 consecutive spaces with a tab. You have just > used spaces and/or not the tabstop at 8 so your code is various indent- > levels higher than the rest which make it harder to read. > * The code is reused for the depends command which breaks with your patch. > Attached is a testcase you can copy to test/integration in the source tree > which will run the tools build in this tree, so you can test your patch > without installing anything, you just need to build the tree and run the > testcase(s). This will show you the next two things: > * You are printing your s string to often (you don't need it, just print it > as the code did it before and move the duplication check before it > instead) > * If the DependencyType is shown you should disable the duplication check as > otherwise only one of possible many dependencies is shown. > > In general, it seems to be a burden at first, but writing a testcase for the > issue at hand can save you from a lot of problems later on. > > Hope that helps. > > > Best regards > > David Kalnischkies > -- Regards, Ishan Jayawardena. -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
