Hi Nikhil, I had a superficial look: * I still recommend an autoformatter because it eliminates a certain class of error: the formatting nit before merging a PR. Otherwise, you end up micromanaging e.g. spaced () (https://github.com/nikramakrishnan/freetype-docwriter/blob/master/content.py#L113) and unspaced () (https://github.com/nikramakrishnan/freetype-docwriter/blob/master/content.py#L143) * I also suggest you keep to testing 2.7 and 3.6 or 3.7. Is there a requirement for testing 3.4 and 3.5? * In e.g. formatter.py, you duplicate the module docstring in the comment above. This is redundant, I recommend writing everything important into the docstring, as that is accessible in Python. * Any reason for the "# eof“ comments at the bottom? * I suggest you fix the `import *` statements and instead be explicit. * You might want to use the argparse module instead of getopt, that should simplify docwriter.py significantly. https://docs.python.org/3/howto/argparse.html. Writing your own usage() is inelegant.
As a very general aside, I suggest you avoid doing anything other than assigning arguments in __init__ and instead use @classmethod constructors for initializing everything you need for a class. See e.g. http://as.ynchrono.us/2014/12/asynchronous-object-initialization.html. Using the new @dataclass or the attrs module will nudge you into this direction by writing __init__ for you. I’m not saying you should do this right now, as this requires re-designing most classes. But take it as a suggestion for future work. The general pattern would be: ``` class Formatter(object): def __init__(self, processor, identifiers, chapters, sections, block_index): self.processor = processor self.identifiers = identifiers self.chapters = chapters self.sections = sections self.block_index = block_index @classmethod def with_processor(processor): # construct all that's needed return cls(processor, identifiers, chapters, sections, block_index) # … # Instantiation: formatter = Formatter.with_processor(…) ``` _______________________________________________ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel