Re: [Libguestfs] [PATCH 0/3] New API: find_inode

On Thursday, 25 August 2016 16:05:47 CEST NoxDaFox wrote: > 2016-08-25 14:09 GMT+03:00 Pino Toscano <ptoscano(a)redhat.com&gt;: > > > On Wednesday, 24 August 2016 23:59:53 CEST Matteo Cafasso wrote: > > > The find_inode API allows the User to search all the entries referring > > > to a given inode and returns a tsk_dirent structure for each of them. > > > > > > As I didn't want to change unrelated code, there is a little bit > > > of code duplication at the moment. Plan is to refactor the logic > > > in a dedicated set of patches. > > > > The general idea looks ok, but I'd rather see the duplication dealt > > with sooner than later. > > > In the previous submissions, non related changes were rejected therefore I > thought that was the custom. I don't see how the two cases are the same: what was "rejected" was a single patch contaning both additions documented in the commit message, and unrelated changes such as formatting fixes. I remember to have said that it's a no-go as *single* patch, but they can be sent (and integrated) as different commits.

In this case, refactoring and de-duplication of code should be done in different commits as well.

> Moreover I'll add another API find_block (block_number --> tsk_dirents > referring to it) and I think is easier to refactor the code once all the > use cases are in place as the picture gets more clear. More reasons to refactor *before*: since you already planned more APIs, it's easier to just refactor one in advance with all the common code needed, and use it later. Also, it eases a lot the review of the patches, since it will be less code added.

-- Pino Toscano _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://www.redhat.com/mailman/listinfo/libguestfs