[Asterisk-code-review] release summary: Refactor module to support Git (repotools[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: release_summary: Refactor module to support Git
......................................................................


Patch Set 5: Code-Review-1

(3 comments)

Wow, generating the HTML programatically is sooooo much nicer.

https://gerrit.asterisk.org/#/c/179/5/release_summary.py
File release_summary.py:

Line 126:        n_type = ReleaseSummaryOptions.str_to_release_type.get(release_type)
        :         if n_type:
        :             return n_type
        :         raise Exception("Unknown release type: {0}".format(release_type))
Instead of using .get here, you could use the square bracket notation here. If n_type isn't in the dict, then a KeyError will be thrown and you can catch that when calling get_release_type instead of just catching a generic Exception type.

I think it's really just a matter of taste if you want to change it to that or not, so feel free to ignore this comment.


Line 258:             if log_message.raw and len(log_message.raw.parents) > 1:
        :                 # Ignore merge commits
        :                 continue
You should update pbar here.


Line 279:            if len(issues) == 0:
        :                 self.misc_commits.append(log_message)
        :                 continue
update pbar here.


-- 
To view, visit https://gerrit.asterisk.org/179
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I97275322fe5b8067e2c7317b6fbbfab532e6cd48
Gerrit-PatchSet: 5
Gerrit-Project: repotools
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes