On 02/13/2017 01:24 PM, Jim Fehlig wrote: > Sagar Ghuge wrote: >> Hi Jim, >> >> Thank you for replying. I am planning to participate in GSoC'17 and >> this is my first patch towards it. This is from Byte sized task which >> I picked up and trying to solve it. > > Ah, yes, this one > > http://wiki.libvirt.org/page/BiteSizedTasks#Add_function_that_raises_error_if_domain_is_not_active > >> Yes you are right, pattern is used >> throughout the libxl driver and many others but this is just a >> rudimentary patch which will help me to get suggestion that am I on >> right path. if yes then I can I can go ahead and make changes >> accordingly. > > It looks good to me, although I'm not sure how to best split up the work. Maybe > a patch that adds virDomainObjCheckIsActive, and then a patch for each driver > that replaces virDomainObjIsActive with virDomainObjCheckIsActive? > > AFAICT, Cole added that task to the wiki. Perhaps he has a suggestion on the > best way to organize the work. > Sorry for the late response. For these types of patches, please link to the BiteSizedTask in the mail after the -- break generated by git format-patch: this keeps the link out of the commit message since it's not really relevant there, but gives more info to potential patch reviewers. For this item I'd say do a patch series containing more instances of conversions, to show that it's actually simplifying a common code pattern. So maybe a patch series like: 1) Add the function 2) Convert src/test/* 3) Convert src/libxl/* 4) Convert src/xen/* That shouldn't be too much work and will give a bit more to comment on, and if there's no objections, send the remaining conversions as an additional patch series. - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
