Closed salmannotkhan closed 4 years ago
I added another commit to make code clean and easy to work with
@salmannotkhan -- thanks for your PR. I've review it and outline improvements below. I have forked from your first commit into a separate branch/PR see here.
listnm
and newname
. Please use descriptive variable names in future PRs to simplify this.generate_names
method in particular has code smells
as the same logic exists within and outside the IF/ELSE (see here, e.g. newname.append(listnm[i])
). This indicates that this could have been refactored to simplify the logic -- I appreciate that at the time you may not have known a better solution, but we are no in rush 👍 /src/
as it is better to be explicit on what the package is named (i.e. csv2docx). Furthermore, these specific changes introduced import errors and poetry run convert
no longer works..seek(1)
as you have implemented here would have been ideal.I have branched from your first commit and made changes to simplify the logic as outlined above -- see #20.
now can use custom naming using -n parameter -n [column_name]