Review and update TODOs
This commit is contained in:
Родитель
3baf93494e
Коммит
c4734cd575
|
@ -17,7 +17,6 @@ from comma.database.model import Base
|
|||
from comma.util import config
|
||||
|
||||
|
||||
# TODO: Rename this class because it conflicts with the module name.
|
||||
class DatabaseDriver:
|
||||
"""
|
||||
Database driver managing connections
|
||||
|
|
|
@ -27,18 +27,18 @@ class PatchData(Base):
|
|||
author = Column(String)
|
||||
authorEmail = Column(String)
|
||||
authorTime = Column(DateTime())
|
||||
# TODO: What about committer and their email?
|
||||
# TODO (Issue 40): What about committer and their email?
|
||||
commitTime = Column(DateTime())
|
||||
# TODO: Should we have a filenames table?
|
||||
# TODO (Issue 40): Should we have a filenames table?
|
||||
affectedFilenames = Column(String)
|
||||
commitDiffs = Column(String)
|
||||
# TODO: Should we have a symbols table?
|
||||
# TODO (Issue 40): Should we have a symbols table?
|
||||
symbols = Column(String)
|
||||
# TODO: Should this reference a patchID?
|
||||
# TODO (Issue 40): Should this reference a patchID?
|
||||
fixedPatches = Column(String)
|
||||
# TODO: If this 1-1, why isn't `priority` just a column on `PatchData`?
|
||||
# TODO (Issue 40): If this 1-1, why isn't `priority` just a column on `PatchData`?
|
||||
metaData = relationship("PatchDataMeta", uselist=False, back_populates="patch")
|
||||
# TODO: If this 1-1, why isn't `status` just a column on `PatchData`?
|
||||
# TODO (Issue 40): If this 1-1, why isn't `status` just a column on `PatchData`?
|
||||
upstreamStatus = relationship("UpstreamPatchStatuses", uselist=False, back_populates="patch")
|
||||
monitoringSubject = relationship(
|
||||
"MonitoringSubjectsMissingPatches",
|
||||
|
@ -74,7 +74,7 @@ class Distros(Base):
|
|||
Downstream distro and URL for downstream repo
|
||||
"""
|
||||
|
||||
# TODO: Rename this class and table to "Distro"
|
||||
# TODO (Issue 40): Rename this class and table to "Distro"
|
||||
__tablename__ = "Distros"
|
||||
distroID = Column(String, primary_key=True)
|
||||
repoLink = Column(String)
|
||||
|
@ -103,7 +103,7 @@ class MonitoringSubjectsMissingPatches(Base):
|
|||
Patches missing for a given monitoring subject
|
||||
"""
|
||||
|
||||
# TODO: Rename this table.
|
||||
# TODO (Issue 40): Rename this table.
|
||||
__tablename__ = "MonitoringSubjectsMissingPatches"
|
||||
monitoringSubjectID = Column(
|
||||
Integer, ForeignKey("MonitoringSubjects.monitoringSubjectID"), primary_key=True
|
||||
|
|
|
@ -92,7 +92,7 @@ def patch_matches(downstream_patches: List[PatchData], upstream: PatchData) -> b
|
|||
) >= CONFIDENCE_THRESHOLD:
|
||||
return True
|
||||
|
||||
# TODO just do this part?...
|
||||
# TODO (Issue 53): just do this part?
|
||||
# Check for code matching
|
||||
upstream_diffs = PatchDiff(upstream.commitDiffs)
|
||||
return any(
|
||||
|
|
|
@ -62,10 +62,9 @@ def update_tracked_revisions(distro_id, repo):
|
|||
|
||||
repo: the git repo object of whatever repo to check revisions in
|
||||
"""
|
||||
|
||||
# TODO This is WRONG. It's sorting alphabetically, which happens to be correct currently but
|
||||
# needs to be addressed git tag can sort by date, but can't specify remote.
|
||||
# ls-remote can naturally sort by date, but requires the object to be local
|
||||
# This sorts alphabetically and not by the actual date
|
||||
# While technically wrong, this is preferred
|
||||
# ls-remote could naturally sort by date, but that would require all the objects to be local
|
||||
|
||||
if distro_id.startswith("Ubuntu"):
|
||||
tag_names = []
|
||||
|
@ -199,7 +198,6 @@ def fetch_remote_ref(repo: git.Repo, name: str, local_ref: str, remote_ref: str)
|
|||
remote = repo.remote(name)
|
||||
|
||||
# Initially fetch revision at depth 1
|
||||
# TODO: Is there a better way to get the date of the last commit?
|
||||
logging.info("Fetching remote ref %s from remote %s at depth 1", remote_ref, remote)
|
||||
fetch_info = remote.fetch(remote_ref, depth=1, verbose=True, progress=GitProgressPrinter())
|
||||
|
||||
|
@ -267,7 +265,7 @@ def monitor_downstream():
|
|||
with DatabaseDriver.get_session() as session:
|
||||
for subject in session.query(MonitoringSubjects).all():
|
||||
if subject.distroID.startswith("Debian"):
|
||||
# TODO don't skip Debian
|
||||
# TODO (Issue 51): Don't skip Debian
|
||||
logging.debug("skipping Debian")
|
||||
continue
|
||||
|
||||
|
|
|
@ -119,16 +119,13 @@ def process_commits(
|
|||
all_patches = []
|
||||
num_patches_added = 0
|
||||
for num, commit in enumerate(commits, 1):
|
||||
# First ever commit, we don't need to store this as it'll be present in any distro
|
||||
# TODO revisit, maybe check against set hash of first commit?
|
||||
# Get code some other way? Unsure if first commit matters or not.
|
||||
# Skip root (initial) commit since it should always be present
|
||||
# TODO (Issue 54): This can probably be removed
|
||||
if commit.parents:
|
||||
logging.debug("Parsing commit %s", commit.hexsha)
|
||||
patch: PatchData = create_patch(commit)
|
||||
|
||||
if add_to_database:
|
||||
# TODO is this check needed if we start on only patches we haven't processed before?
|
||||
# If we DO want to keep this check, let's move before parsing everything
|
||||
with DatabaseDriver.get_session() as session:
|
||||
if (
|
||||
session.query(PatchData.commitID)
|
||||
|
|
|
@ -84,7 +84,7 @@ def import_commits(in_file: str) -> None:
|
|||
"""
|
||||
print(f"Sorry, importing is not supported at this time! filename: {in_file}")
|
||||
sys.exit(1)
|
||||
# TODO: Fix tracking to support commits which are manually added
|
||||
# TODO (Issue 55): Implement import from database
|
||||
# to the database, and therefore affect untracked paths.
|
||||
# from comma.upstream import process_commits
|
||||
# print(f"Importing commits from spreadsheet '{in_file}'...")
|
||||
|
@ -135,7 +135,7 @@ def get_release(sha: str, repo: git.Repo) -> str:
|
|||
def create_commit_row(sha: str, repo: git.Repo, worksheet: Worksheet) -> Dict[str, Any]:
|
||||
"""Create a row with the commit's SHA, date, release and title."""
|
||||
commit = repo.commit(sha)
|
||||
# TODO: Some (but not all) of this info is available in the
|
||||
# TODO (Issue 40): Some (but not all) of this info is available in the
|
||||
# database, so if add the release to the database we can skip
|
||||
# using the commit here.
|
||||
date = datetime.utcfromtimestamp(commit.authored_date).date()
|
||||
|
@ -186,7 +186,6 @@ def export_commits(in_file: str, out_file: str) -> None:
|
|||
# worksheet.
|
||||
print(f"Exporting {len(missing_commits)} commits to {out_file}...")
|
||||
for commit in missing_commits:
|
||||
# TODO: Set fonts of the cells.
|
||||
worksheet.append(create_commit_row(commit, repo, worksheet))
|
||||
|
||||
workbook.save(out_file)
|
||||
|
@ -196,7 +195,7 @@ def export_commits(in_file: str, out_file: str) -> None:
|
|||
def get_distros() -> List[str]:
|
||||
"""Collect the distros we’re tracking in the database."""
|
||||
with DatabaseDriver.get_session() as session:
|
||||
# TODO: Handle Debian.
|
||||
# TODO (Issue 51): Handle Debian.
|
||||
return [
|
||||
distro
|
||||
for (distro,) in session.query(Distros.distroID)
|
||||
|
@ -227,8 +226,8 @@ def get_revision(distro: str, commit: str, commits: Dict[str, int]) -> str:
|
|||
.one()
|
||||
)
|
||||
|
||||
# TODO: We could try to simplify this using the ‘monitoringSubject’ relationship on the
|
||||
# ‘PatchData’ table, but because the database tracks what’s missing, it becomes
|
||||
# TODO (Issue 40): We could try to simplify this using the ‘monitoringSubject’ relationship
|
||||
# on the ‘PatchData’ table, but because the database tracks what’s missing, it becomes
|
||||
# hard to state where the patch is present.
|
||||
missing_patch = subject.missingPatches.filter_by(patchID=commits[commit]).one_or_none()
|
||||
|
||||
|
|
|
@ -103,7 +103,7 @@ class Session:
|
|||
return self.repos[name]
|
||||
|
||||
|
||||
# TODO: Move session creation to main program logic
|
||||
# TODO (Issue 56): Move session creation to main program logic
|
||||
SESSION = Session()
|
||||
|
||||
|
||||
|
|
|
@ -94,7 +94,6 @@ ignore-long-lines = "^\\s*(# )?<?https?://\\S+>?$|^\\s*([^=]+=)?[f|r]?b?[\\\"\\'
|
|||
min-public-methods = 1
|
||||
|
||||
[tool.pylint."messages control"]
|
||||
disable = ["fixme"]
|
||||
enable = [
|
||||
"bad-inline-option",
|
||||
"c-extension-no-member",
|
||||
|
@ -130,6 +129,14 @@ load-plugins = [
|
|||
# version used to run pylint.
|
||||
py-version = "3.8"
|
||||
|
||||
[tool.pylint.miscellaneous]
|
||||
# List of note tags to take in consideration, separated by a comma
|
||||
# See https://github.com/pylint-dev/pylint/issues/8734
|
||||
notes=["SomethingThatWillNotBeFound"]
|
||||
|
||||
# Regular expression of note tags to take in consideration.
|
||||
notes-rgx="TODO(?! \\(Issue \\d+\\))"
|
||||
|
||||
[tool.pylint.reports]
|
||||
output-format = "colorized"
|
||||
|
||||
|
|
Загрузка…
Ссылка в новой задаче