Re: [StGIT PATCH 3/5] Add stack creation and initialisation support to lib.Stack

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/06/2008, Karl Hasselström <kha@xxxxxxxxxxx> wrote:
> On 2008-06-06 21:45:54 +0100, Catalin Marinas wrote:
>  > +    @staticmethod
>  > +    def create(stackdir):
>  > +        """Create the PatchOrder specific files
>  > +        """
>  > +        utils.create_empty_file(os.path.join(stackdir, 'applied'))
>  > +        utils.create_empty_file(os.path.join(stackdir, 'unapplied'))
>  > +        utils.create_empty_file(os.path.join(stackdir, 'hidden'))
>
>
> Maybe s/create/create_files/ or soething to that effect here, as I
>  suggested in another mail a few minutes ago.

Done, no problem with that.

>  > +    def set_parents(self, remote, branch):
>  > +        if remote:
>  > +            self.set_parent_remote(remote)
>  > +        if branch:
>  > +            self.set_parent_branch(branch)
>
>
> Much clearer now. Interesting that the only reason we need this
>  function at all is to make sure that set_parent_remote and
>  set_parent_branch are called in the correct order.
>
>  Maybe set_parent_branch should throw an exception instead of silently
>  doing nothing when there is no parent yet? It would force the callers
>  to be more aware of the limitation -- though we might not actually
>  want that. Your call.

I can't argue much. I propose to commit the patch as is and modify it
afterwards. I haven't touched this code much, it was Yann's
implementation and I forgot all the decisions at that time.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux