[snip]

Quoting Jason Ekstrand (2016-06-08 16:20:33)
> gen_format_layout.c.mako
> 
> I made this comment in the office today but I think the mako here is simple
> enough that we might be better off just putting it all in one file.
> 

I know you're not a fan of separate templates, and if the consensus is
to put the template in the python file I'll do that, but my experience
working with mako makes me wary of doing that for templates over a few
lines. I think they're very hard to read (remember that the copyright
header is part of the template), and you don't get syntax highlighting
which makes them much harder to work with.

[snip]

> > +def reader():
> > +    """Wrapper around csv.reader that skips comments and blanks."""
> > +    # csv.reader actually reads the file one line at a time (it was 
> > designed
> to
> > +    # open excel generated sheets), so hold the file until all of the lines
> are
> > +    # read.
> > +    with open('isl_format_layout.csv', 'r') as f:
> 
> I'm not so sure this works.  It's probably better to pass the file name in.  
> Maybe this is safe but I'm skeptical.

I don't know, I can change it, it just seemed kinda silly to add command
parsing to a generator that reads exactly one file.

Maybe someone with more autotools experience could say whether this is a
good idea or not?

> 
> Other than those two comments, this seems perfectly reasonable.
> 

Okay, I'll wait for some more feedback and send out a v2.

[snip]

Dylan

Attachment: signature.asc
Description: signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to