Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(228)

Side by Side Diff: presubmit_canned_checks.py

Issue 1928343002: Revert of Implement owners check in presubmit for Gerrit. (Closed) Base URL: https://chromium.googlesource.com/chromium/tools/depot_tools.git@P300
Patch Set: Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « git_cl.py ('k') | presubmit_support.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 """Generic presubmit checks that can be reused by other presubmit checks.""" 5 """Generic presubmit checks that can be reused by other presubmit checks."""
6 6
7 import os as _os 7 import os as _os
8 _HERE = _os.path.dirname(_os.path.abspath(__file__)) 8 _HERE = _os.path.dirname(_os.path.abspath(__file__))
9 9
10 # Justifications for each filter: 10 # Justifications for each filter:
(...skipping 857 matching lines...) Expand 10 before | Expand all | Expand 10 after
868 needed = 'LGTM from an OWNER' 868 needed = 'LGTM from an OWNER'
869 output = output_api.PresubmitError 869 output = output_api.PresubmitError
870 else: 870 else:
871 needed = 'OWNER reviewers' 871 needed = 'OWNER reviewers'
872 output = output_api.PresubmitNotifyResult 872 output = output_api.PresubmitNotifyResult
873 873
874 affected_files = set([f.LocalPath() for f in 874 affected_files = set([f.LocalPath() for f in
875 input_api.change.AffectedFiles(file_filter=source_file_filter)]) 875 input_api.change.AffectedFiles(file_filter=source_file_filter)])
876 876
877 owners_db = input_api.owners_db 877 owners_db = input_api.owners_db
878 owner_email, reviewers = _CodereviewOwnersAndReviewers( 878 # TODO(tandrii): this will always return None, set() in case of Gerrit.
879 owner_email, reviewers = _RietveldOwnerAndReviewers(
879 input_api, 880 input_api,
880 owners_db.email_regexp, 881 owners_db.email_regexp,
881 approval_needed=input_api.is_committing) 882 approval_needed=input_api.is_committing)
882 883
883 owner_email = owner_email or input_api.change.author_email 884 owner_email = owner_email or input_api.change.author_email
884 885
885 if owner_email: 886 if owner_email:
886 reviewers_plus_owner = set([owner_email]).union(reviewers) 887 reviewers_plus_owner = set([owner_email]).union(reviewers)
887 missing_files = owners_db.files_not_covered_by(affected_files, 888 missing_files = owners_db.files_not_covered_by(affected_files,
888 reviewers_plus_owner) 889 reviewers_plus_owner)
889 else: 890 else:
890 missing_files = owners_db.files_not_covered_by(affected_files, reviewers) 891 missing_files = owners_db.files_not_covered_by(affected_files, reviewers)
891 892
892 if missing_files: 893 if missing_files:
893 output_list = [ 894 output_list = [
894 output('Missing %s for these files:\n %s' % 895 output('Missing %s for these files:\n %s' %
895 (needed, '\n '.join(sorted(missing_files))))] 896 (needed, '\n '.join(sorted(missing_files))))]
896 if not input_api.is_committing: 897 if not input_api.is_committing:
897 suggested_owners = owners_db.reviewers_for(missing_files, owner_email) 898 suggested_owners = owners_db.reviewers_for(missing_files, owner_email)
898 output_list.append(output('Suggested OWNERS: ' + 899 output_list.append(output('Suggested OWNERS: ' +
899 '(Use "git-cl owners" to interactively select owners.)\n %s' % 900 '(Use "git-cl owners" to interactively select owners.)\n %s' %
900 ('\n '.join(suggested_owners or [])))) 901 ('\n '.join(suggested_owners or []))))
901 return output_list 902 return output_list
902 903
903 if input_api.is_committing and not reviewers: 904 if input_api.is_committing and not reviewers:
904 return [output('Missing LGTM from someone other than %s' % owner_email)] 905 return [output('Missing LGTM from someone other than %s' % owner_email)]
905 return [] 906 return []
906 907
907 def _CodereviewOwnersAndReviewers(input_api, email_regexp, approval_needed):
908 """Return the owner and reviewers of a change, if any.
909
910 If approval_needed is True, only reviewers who have approved the change
911 will be returned.
912 """
913 if input_api.change.issue:
914 if input_api.gerrit:
915 res = _GerritOwnerAndReviewers(input_api, email_regexp, approval_needed)
916 else:
917 # Rietveld is default.
918 res = _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed)
919 if res:
920 return res
921
922 reviewers = set()
923 if not approval_needed:
924 reviewers = _ReviewersFromChange(input_api.change)
925 return None, reviewers
926
927 908
928 def _GetRietveldIssueProps(input_api, messages): 909 def _GetRietveldIssueProps(input_api, messages):
929 """Gets the issue properties from rietveld.""" 910 """Gets the issue properties from rietveld."""
930 issue = input_api.change.issue 911 issue = input_api.change.issue
931 if issue and input_api.rietveld: 912 if issue and input_api.rietveld:
932 return input_api.rietveld.get_issue_properties( 913 return input_api.rietveld.get_issue_properties(
933 issue=int(issue), messages=messages) 914 issue=int(issue), messages=messages)
934 915
935 916
936 def _ReviewersFromChange(change): 917 def _ReviewersFromChange(change):
937 """Return the reviewers specified in the |change|, if any.""" 918 """Return the reviewers specified in the |change|, if any."""
938 reviewers = set() 919 reviewers = set()
939 if change.R: 920 if change.R:
940 reviewers.update(set([r.strip() for r in change.R.split(',')])) 921 reviewers.update(set([r.strip() for r in change.R.split(',')]))
941 if change.TBR: 922 if change.TBR:
942 reviewers.update(set([r.strip() for r in change.TBR.split(',')])) 923 reviewers.update(set([r.strip() for r in change.TBR.split(',')]))
943 924
944 # Drop reviewers that aren't specified in email address format. 925 # Drop reviewers that aren't specified in email address format.
945 return set(reviewer for reviewer in reviewers if '@' in reviewer) 926 return set(reviewer for reviewer in reviewers if '@' in reviewer)
946 927
947 928
948 def _match_reviewer_email(r, owner_email, email_regexp):
949 return email_regexp.match(r) and r != owner_email
950
951 def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False): 929 def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
952 """Return the owner and reviewers of a change, if any. 930 """Return the owner and reviewers of a change, if any.
953 931
954 If approval_needed is True, only reviewers who have approved the change 932 If approval_needed is True, only reviewers who have approved the change
955 will be returned. 933 will be returned.
956 Returns None if can't fetch issue properties from codereview.
957 """ 934 """
958 issue_props = _GetRietveldIssueProps(input_api, True) 935 issue_props = _GetRietveldIssueProps(input_api, True)
959 if not issue_props: 936 if not issue_props:
960 return None 937 reviewers = set()
938 if not approval_needed:
939 reviewers = _ReviewersFromChange(input_api.change)
940 return None, reviewers
961 941
962 if not approval_needed: 942 if not approval_needed:
963 return issue_props['owner_email'], set(issue_props['reviewers']) 943 return issue_props['owner_email'], set(issue_props['reviewers'])
964 944
965 owner_email = issue_props['owner_email'] 945 owner_email = issue_props['owner_email']
966 946
947 def match_reviewer(r):
948 return email_regexp.match(r) and r != owner_email
949
967 messages = issue_props.get('messages', []) 950 messages = issue_props.get('messages', [])
968 approvers = set( 951 approvers = set(
969 m['sender'] for m in messages 952 m['sender'] for m in messages
970 if m.get('approval') and _match_reviewer_email(m['sender'], owner_email, 953 if m.get('approval') and match_reviewer(m['sender']))
971 email_regexp)) 954
972 return owner_email, approvers 955 return owner_email, approvers
973 956
974 957
975 def _GerritOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
976 """Return the owner and reviewers of a change, if any.
977
978 If approval_needed is True, only reviewers who have approved the change
979 will be returned.
980 Returns None if can't fetch issue properties from codereview.
981 """
982 issue = input_api.change.issue
983 if not issue:
984 return None
985
986 owner_email = input_api.gerrit.GetChangeOwner(issue)
987 reviewers = set(
988 r for r in input_api.gerrit.GetChangeReviewers(issue, approval_needed)
989 if _match_reviewer_email(r, owner_email, email_regexp))
990 return owner_email, reviewers
991
992
993 def _CheckConstNSObject(input_api, output_api, source_file_filter): 958 def _CheckConstNSObject(input_api, output_api, source_file_filter):
994 """Checks to make sure no objective-c files have |const NSSomeClass*|.""" 959 """Checks to make sure no objective-c files have |const NSSomeClass*|."""
995 pattern = input_api.re.compile( 960 pattern = input_api.re.compile(
996 r'(?<!reinterpret_cast<)' 961 r'(?<!reinterpret_cast<)'
997 r'const\s+NS(?!(Point|Range|Rect|Size)\s*\*)\w*\s*\*') 962 r'const\s+NS(?!(Point|Range|Rect|Size)\s*\*)\w*\s*\*')
998 963
999 def objective_c_filter(f): 964 def objective_c_filter(f):
1000 return (source_file_filter(f) and 965 return (source_file_filter(f) and
1001 input_api.os_path.splitext(f.LocalPath())[1] in ('.h', '.m', '.mm')) 966 input_api.os_path.splitext(f.LocalPath())[1] in ('.h', '.m', '.mm'))
1002 967
(...skipping 160 matching lines...) Expand 10 before | Expand all | Expand 10 after
1163 for f in affected_files: 1128 for f in affected_files:
1164 cmd = ['gn', 'format', '--dry-run', f.AbsoluteLocalPath()] 1129 cmd = ['gn', 'format', '--dry-run', f.AbsoluteLocalPath()]
1165 rc = gn.main(cmd) 1130 rc = gn.main(cmd)
1166 if rc == 2: 1131 if rc == 2:
1167 warnings.append(output_api.PresubmitPromptWarning( 1132 warnings.append(output_api.PresubmitPromptWarning(
1168 '%s requires formatting. Please run `gn format --in-place %s`.' % ( 1133 '%s requires formatting. Please run `gn format --in-place %s`.' % (
1169 f.AbsoluteLocalPath(), f.LocalPath()))) 1134 f.AbsoluteLocalPath(), f.LocalPath())))
1170 # It's just a warning, so ignore other types of failures assuming they'll be 1135 # It's just a warning, so ignore other types of failures assuming they'll be
1171 # caught elsewhere. 1136 # caught elsewhere.
1172 return warnings 1137 return warnings
OLDNEW
« no previous file with comments | « git_cl.py ('k') | presubmit_support.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698