-
Notifications
You must be signed in to change notification settings - Fork 247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft: Add pr-check for forked project #334
base: main
Are you sure you want to change the base?
Changes from all commits
6be8abb
5835255
acebe99
e62acb7
abd980c
a2c0d8d
c3e72c8
5528727
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
import re | ||
import csv | ||
import subprocess | ||
import requests | ||
from utils import ( | ||
get_committed_lines, | ||
get_uncommitted_lines, | ||
|
@@ -14,7 +15,7 @@ | |
|
||
# Contains regexes for columns that are commmon to pr-data and tic-fic-data | ||
common_data = { | ||
"Project URL": re.compile(r"(https:\/\/github.com)(\/(\w|\.|-)+){2}"), | ||
"Project URL": re.compile(r"(https:\/\/github.com\/([\w|\.|-]+)\/([\w|\.|-]+))"), | ||
"SHA": re.compile(r"\b[0-9a-f]{40}\b"), | ||
"Module Path": re.compile(r"((\w|\.|-)+(\/|\w|\.|-)*)|^$"), | ||
"Fully-Qualified Name": re.compile( | ||
|
@@ -23,6 +24,27 @@ | |
} | ||
|
||
|
||
def check_repo_sanity(checked_projects, filename, row, i, log): | ||
project_url = row["Project URL"] | ||
if project_url in checked_projects: | ||
return | ||
checked_projects.add(project_url) | ||
|
||
match = common_data["Project URL"].fullmatch(project_url) | ||
author = match.group(2) | ||
repo = match.group(3) | ||
|
||
url = "https://api.github.com/repos/{}/{}".format(author, repo) | ||
try: | ||
resp = requests.get(url).json() | ||
# Determine if it is a forked project | ||
if resp.get("fork"): | ||
log_esp_error(filename, log, f"{author}/{repo} is a forked repo") | ||
except requests.exceptions.RequestException as e: | ||
# handle(e) | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of simply passing here, it would be good to warn that the check for fork failed along with outputting the exception |
||
|
||
|
||
def check_header(header, valid_dict, filename, log): | ||
"""Validates that the header is correct.""" | ||
|
||
|
@@ -108,6 +130,7 @@ def run_checks(file, data_dict, log, commit_range, checks): | |
if "1" in uncommitted_lines or "1" in committed_lines: | ||
check_header(list(header.values()), data_dict, file, log) | ||
if uncommitted_lines != [] or committed_lines != []: | ||
checked_projects = set() | ||
for i, row in enumerate(info): | ||
i += 2 | ||
line = str(i) | ||
|
@@ -123,6 +146,9 @@ def run_checks(file, data_dict, log, commit_range, checks): | |
if check_rule.__name__ == check_row_length.__name__: | ||
check_rule(len(header), *params) | ||
continue | ||
if check_rule.__name__ == check_repo_sanity.__name__: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of running this check_rule for every changed line in the CSV file, we could just run this check when a new Project URL is added that does not already exist. i.e., get (1) the Project URLs that are already in the unchanged file and (2) the Project URLs in the new changes, and only run this check for Project URLs that are in (2) but not in (1). This improvement need not be done immediately but would help avoid the Github limit. |
||
check_rule(checked_projects, *params) | ||
continue | ||
check_rule(*params) | ||
else: | ||
log_info(file, log, "There are no changes to be checked") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
errorhandler==2.0.1 | ||
requests==2.18.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the unchanged line giving you some issues?