Alon Bar-Lev has posted comments on this change. Change subject: engine: refactor: add Model attribute for help tagging ......................................................................
Patch Set 11: (8 comments) http://gerrit.ovirt.org/#/c/21052/11/build/helptag.py File build/helptag.py: Line 43: __RE_COMMENT = re.compile( Line 44: flags=re.VERBOSE, Line 45: pattern=r""" Line 46: .* Line 47: @Comment once again.. single RE with (@Command|@Name) should be sufficient Line 48: \s* Line 49: \( Line 50: \s*"\s* Line 51: (?P<comment>[^")]*) Line 78: tags = {} Line 79: buffer = "" Line 80: Line 81: if filename.endswith('.java') and os.path.isfile(filename): Line 82: with open(filename, 'r') as f: try: with open(filename) as f: content = f.read() for block in content.split(';'): for s in _RE.findall(block): m = _RE.match(s) # whatever python does not have 'next' method for matcher... bad them.... but no need to do the ugly loop. Line 83: for line in f: Line 84: if not line.startswith('import'): Line 85: line = line.strip() Line 86: buffer += " " + line http://gerrit.ovirt.org/#/c/21052/11/build/helptag_checker.py File build/helptag_checker.py: Line 47: Line 48: __RE_QUOTED_STRING = re.compile( Line 49: flags=re.VERBOSE, Line 50: pattern=r'"[^"]*"' Line 51: ) why not embed this re into the above ones? Line 52: Line 53: Line 54: def walkSource(sourcedir): Line 55: """ Line 74: if m: Line 75: hashname = m.group("hashname") Line 76: m2 = __RE_QUOTED_STRING.match(hashname) Line 77: if not m2: Line 78: message = message + ("BAD LINE: %s" % line) message += "BAD LINE: %s" % line Line 79: Line 80: if (message != ""): Line 81: print fname Line 82: print message Line 76: m2 = __RE_QUOTED_STRING.match(hashname) Line 77: if not m2: Line 78: message = message + ("BAD LINE: %s" % line) Line 79: Line 80: if (message != ""): if message: Line 81: print fname Line 82: print message Line 83: Line 84: Line 78: message = message + ("BAD LINE: %s" % line) Line 79: Line 80: if (message != ""): Line 81: print fname Line 82: print message please use python3 notation Line 83: Line 84: Line 85: def findHashNamesWithoutHelpTag(sourcedir): Line 86: for (lines, fname) in walkSource(sourcedir): Line 84: Line 85: def findHashNamesWithoutHelpTag(sourcedir): Line 86: for (lines, fname) in walkSource(sourcedir): Line 87: message = "" Line 88: for i in range(0, len(lines)): please move to enumerate or better parse up to ';' each iteration Line 89: m = __RE_SETHASHNAME.match(lines[i]) Line 90: if m: Line 91: hashname = m.group("hashname").replace('"', '') \ Line 92: .replace('-', '_') Line 116: Line 117: for helptag in tags.keys(): Line 118: files = tags[helptag] Line 119: if (len(files) > 1): Line 120: print("DUPLICATE HELPTAG CALL:\n%s\n%s\n" % (helptag, files)) try: sys.stdout.write( ( "DUPLICATE HELPTAG CALL:\n" "%s\n" "%s\n" ) % ( helptag, files, ) ) but why capitals? WARNING: message should be sufficient. so add: def warning(s): sys.stdout.write("WARNING: " + s) and call it from both places Line 121: Line 122: Line 123: def main(): Line 124: parser = argparse.ArgumentParser( -- To view, visit http://gerrit.ovirt.org/21052 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia4074fcc2ecfcbdd2ea6c0855d92f2aa4bd26a5b Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
