Closed groteworld closed 7 years ago
Totally agree and updated.
I'm going to test this and accept it. I might take this a slightly different direction (see also https://youtu.be/OMPfEXIlTVE).
My eyes are open! So you're thinking something closer to....
csv.py
def csv_writer():
# csv writing code
def main():
Converter('csv', '.csv', csv_writer)
converter.py
class Converter():
def __init__(self, name='BaseConverter', extension='.txt', writer=self.default_writer, **kwargs):
self.name = name
self.extension = extension
self.writer = writer
# ....
self.writer()
def default_writer(self):
# default writer stuff
wrote this in this comment so unsure on "truthiness"
So I think we have two things we care about (right now)
I don't think we should conflate the two. The Writer
class (whatever we call it) can handle default file names, file extensions (by proxy), and handling the file-pointer. The Formatter
class can receive the issues and format them appropriately.
I think both classes will need to be able to register arguments on the ArgumentParser
instance, so we'll have something like:
class Converter(object):
def __init__(self, formatter=None, writer=None):
if formatter is None:
raise ValueError('..')
if writer is None:
raise ValueError('..')
self.formatter = formatter
self.writer = writer
self.parser = self.make_parser()
# These next two lines can be in their own method too
self.formatter.register_arguments(self.parser)
self.writer.register_arguments(self.parser)
self.args = self.parser.parse_args()
self.make_client() # Setups up our instance of github3.GitHub
If we want, make_parser
can read information from self.writer
by either using it implicitly or by explicitly passing it in. Make sense?
Also, don't feel as if you have to do this. I'm getting paid to work on this for now =P
NP, just helping out the discussion.
And I appreciate it :)
This commit adds a base converter that contains all the common functions used by all converters. New converters simple can inherit from the BaseConverter class and add the following:
Each child class has access to the variables and functions in base converter such as, but not limited to:
This could be made even more concise by having the base return back a list of issues based off of the args, and not requirings the use of the args variable or helper functions like is_pull_request, but I think that doesn't allow for opinionated configurations.