Skip to content

Streamline main script.#12

Open
ben-e-whitney wants to merge 1 commit intogabru-md:masterfrom
ben-e-whitney:dev
Open

Streamline main script.#12
ben-e-whitney wants to merge 1 commit intogabru-md:masterfrom
ben-e-whitney:dev

Conversation

@ben-e-whitney
Copy link
Copy Markdown
Contributor

Notes:

  • The newlines in the description are ignored. See the documentation. For the same reason I have edited the help parameters for the arguments. You can use formatters if you want to put in the newlines manually.
  • Since you aren't in any way handling any exception that may be raised when appending to members and names, I think it's better to just let the exception raise. Then the user will at least see what the type of the exception is.
  • String concatenation was working fine, but I believe it's considered better practice to use os.path.join.
  • Rather than iterating over range(len(replace[uid])) (AKA range(len(members))), we can directly iterate over zip(members, names).
  • By using infile.seek(0), we can avoid reopening the file for every contributor.
  • If a member hasn't given their name, the corresponding entry of names will be None and the line.replace call will fail. Using DEFAULT_NAME as a default avoids this.
  • Providing the exit message to sys.exit causes a nonzero exit code to be returned to the shell. See the documentation. We can instead just print the message and exit normally.

Let me know if anything else is unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant