Re: [PATCH spice-common] codegen: Removed unused get_type methods

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

 



> 
> On Wed, 2018-05-09 at 05:10 -0400, Frediano Ziglio wrote:
> > > 
> > > On Sun, 2018-05-06 at 13:29 +0100, Frediano Ziglio wrote:
> > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > > ---
> > > >  python_modules/ptypes.py | 9 ---------
> > > >  1 file changed, 9 deletions(-)
> > > > 
> > > > diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
> > > > index d29c97a..209c00e 100644
> > > > --- a/python_modules/ptypes.py
> > > > +++ b/python_modules/ptypes.py
> > > > @@ -145,9 +145,6 @@ class Type:
> > > >      def has_name(self):
> > > >          return self.name != None
> > > >  
> > > > -    def get_type(self, recursive=False):
> > > > -        return self
> > > > -
> > > >      def is_primitive(self):
> > > >          return False
> > > >  
> > > > @@ -256,12 +253,6 @@ class TypeAlias(Type):
> > > >          self.the_type = the_type
> > > >          self.attributes = fix_attributes(attribute_list)
> > > >  
> > > > -    def get_type(self, recursive=False):
> > > > -        if recursive:
> > > > -            return self.the_type.get_type(True)
> > > > -        else:
> > > > -            return self.the_type
> > > > -
> > > >      def primitive_type(self):
> > > >          return self.the_type.primitive_type()
> > > 
> > > It seems these methods might be there for "API completeness" of the
> > > classes and therefore perhaps better to keep them there (though I
> > > didn't really go and see how much sense it makes). But if you
> > > insist,
> > > 
> > > Acked-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > > 
> > > (might also wanna wait a bit to see if someone holds special
> > > feelings
> > > for them :))
> > > 
> > 
> > I found confusing that a Type class has a "get_type" method :-)
> > Looking around looks like this maybe was replaced by "resolve" method
> > which find the right concrete type.
> > 
> > Frediano
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
> For changes to the code generation scripts, I would find it useful to
> provide a diff between the generated code. I know it's extra work for
> the author, but it would make it a bit easier for the reviewer to
> understand the impact of the change. (I guess in this case, there would
> be no diff, right?)

Yes, being unused in this case nothing changed in the generated code.
Does not take much to get a diff before and after, is reasonable, at least
some of the diff if is going to change a lot.

> 
> Also: it would be interesting to know how you realized these were
> unused? In other words: why were you looking at this code?
> 

Discussing with Lukash I was trying to understand a way to extend a message
and I was looking at the code. I don't exactly remember why I manage to
finish to look for these methods. Did a grep, nothing. A grep in the full
git history and they were never used (at least in git history).
Trying remove them causes no changes (not that I expected something else,
just making sure everything was fine).
Considering that is at least 8 years that they are not used I removed them.

> Jonathon
> 

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]