mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-23 20:29:06 -05:00
refactor: drop comments that restate the code
Thirty one-line labels in verify.py, generate_site.py and
generate_truth.py named the statement under them ("# Load registry",
"# Copy branding assets"). Comments that give a reason stay, as do the
module section headers. A test reads the three files for new ones.
This commit is contained in:
1 parent
3613c0927b
commit
76e44a74b4
4 files changed
+28
-32
No files matched your search
@@ -379,7 +379,6 @@ def generate_home(
|
||||
]
|
||||
)
|
||||
|
||||
# Emulator classification breakdown
|
||||
lines.extend(
|
||||
[
|
||||
"## Emulator profiles",
|
||||
@@ -413,7 +412,6 @@ def generate_home(
|
||||
]
|
||||
)
|
||||
|
||||
# Quick links
|
||||
lines.extend(
|
||||
[
|
||||
"---",
|
||||
@@ -1950,7 +1948,6 @@ def generate_platform_page(
|
||||
if fname:
|
||||
config_files.setdefault((sys_id, fname), []).append(fe)
|
||||
|
||||
# Group details by system
|
||||
by_system: dict[str, list] = {}
|
||||
for d in cov["details"]:
|
||||
sys_id = d.get("system", "unknown")
|
||||
@@ -2647,7 +2644,6 @@ def generate_system_page(
|
||||
icon_md = system_icon_markdown(manufacturer, console_name)
|
||||
lines.append(f"## {icon_md}{console_name}")
|
||||
lines.append("")
|
||||
# Separate main files from variants
|
||||
main_files = [f for f in files if "/.variants/" not in f["path"]]
|
||||
variant_files = [f for f in files if "/.variants/" in f["path"]]
|
||||
|
||||
@@ -2682,7 +2678,6 @@ def generate_emulators_index(profiles: dict) -> str:
|
||||
}
|
||||
aliases = {k: v for k, v in profiles.items() if v.get("type") == "alias"}
|
||||
|
||||
# Group by classification
|
||||
by_class: dict[str, list[tuple[str, dict]]] = {}
|
||||
for name in sorted(unique.keys()):
|
||||
p = unique[name]
|
||||
@@ -4315,7 +4310,6 @@ def _render_xref_by_platform(
|
||||
sys_str += f" +{len(systems) - 3}"
|
||||
|
||||
file_count = len(files)
|
||||
# Count mode divergences
|
||||
libretro_only = sum(1 for f in files if f.get("mode") == "libretro")
|
||||
standalone_only = sum(1 for f in files if f.get("mode") == "standalone")
|
||||
file_str = str(file_count)
|
||||
@@ -4364,7 +4358,6 @@ def _render_xref_by_upstream(
|
||||
]
|
||||
)
|
||||
|
||||
# Group profiles by upstream
|
||||
by_upstream: dict[str, list[str]] = {}
|
||||
for emu_name, p in sorted(unique.items()):
|
||||
# A null upstream is the project's own source: grouping it under
|
||||
@@ -4374,7 +4367,6 @@ def _render_xref_by_upstream(
|
||||
if up_str:
|
||||
by_upstream.setdefault(up_str, []).append(emu_name)
|
||||
|
||||
# Build platform membership per core
|
||||
platform_membership: dict[str, set[str]] = {}
|
||||
for pname, cov in coverages.items():
|
||||
for key in resolve_platform_cores(cov["config"], unique):
|
||||
@@ -4857,7 +4849,6 @@ def generate_mkdocs_nav(
|
||||
k: v for k, v in profiles.items() if v.get("type") not in ("alias", "test")
|
||||
}
|
||||
|
||||
# Group emulators by classification for nav
|
||||
by_class: dict[str, list[tuple[str, str]]] = {}
|
||||
for name in sorted(unique_profiles.keys()):
|
||||
p = unique_profiles[name]
|
||||
@@ -4865,7 +4856,6 @@ def generate_mkdocs_nav(
|
||||
display = p.get("emulator", name)
|
||||
by_class.setdefault(cls, []).append((display, f"emulators/{name}.md"))
|
||||
|
||||
# Classification display names
|
||||
cls_labels = {
|
||||
"pure_libretro": "Pure libretro",
|
||||
"official_port": "Official ports",
|
||||
@@ -4983,7 +4973,6 @@ def _generate(args: argparse.Namespace) -> None:
|
||||
for d in GENERATED_DIRS:
|
||||
(docs / d).mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Copy stylesheet if source exists
|
||||
css_src = Path("docs_assets") / "extra.css"
|
||||
css_dest = docs / "stylesheets" / "extra.css"
|
||||
if css_src.exists():
|
||||
@@ -5000,7 +4989,6 @@ def _generate(args: argparse.Namespace) -> None:
|
||||
# the sitemap instead (overrides/main.html).
|
||||
(docs / "robots.txt").unlink(missing_ok=True)
|
||||
|
||||
# Copy branding assets
|
||||
images_dest = docs / "assets" / "images"
|
||||
images_dest.mkdir(parents=True, exist_ok=True)
|
||||
assets_src = Path(".github") / "assets"
|
||||
@@ -5047,7 +5035,6 @@ def _generate(args: argparse.Namespace) -> None:
|
||||
f" {len(profiles)} profiles ({unique_count} unique, {len(profiles) - unique_count} aliases)"
|
||||
)
|
||||
|
||||
# Build cross-reference indexes
|
||||
consumers = _build_file_consumers(db, coverages, profiles)
|
||||
emulator_files = _build_emulator_file_index(profiles)
|
||||
|
||||
|
||||
@@ -72,12 +72,10 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace:
|
||||
def main(argv: list[str] | None = None) -> None:
|
||||
args = parse_args(argv)
|
||||
|
||||
# Load registry
|
||||
registry_path = os.path.join(args.platforms_dir, "_registry.yml")
|
||||
with open(registry_path) as f:
|
||||
registry = (yaml_load(f) or {}).get("platforms", {})
|
||||
|
||||
# Load emulator profiles
|
||||
profiles = load_emulator_profiles(args.emulators_dir)
|
||||
|
||||
# Load database (optional)
|
||||
@@ -86,7 +84,6 @@ def main(argv: list[str] | None = None) -> None:
|
||||
if args.db != DEFAULT_DB_FILE or os.path.exists(args.db):
|
||||
db = load_database(args.db)
|
||||
|
||||
# Determine platforms
|
||||
if args.all:
|
||||
platforms = list_registered_platforms(
|
||||
args.platforms_dir,
|
||||
@@ -108,7 +105,6 @@ def main(argv: list[str] | None = None) -> None:
|
||||
output_dir, [name], args.target, args.platforms_dir
|
||||
)
|
||||
os.makedirs(output_dir, exist_ok=True)
|
||||
# Resolve target cores
|
||||
target_cores: set[str] | None = None
|
||||
if args.target:
|
||||
try:
|
||||
|
||||
@@ -144,7 +144,6 @@ class Status:
|
||||
MISSING = "missing"
|
||||
|
||||
|
||||
# Severity for per-file required/optional distinction
|
||||
class Severity:
|
||||
CRITICAL = "critical" # required file missing or bad hash (Recalbox RED)
|
||||
WARNING = "warning" # optional missing or hash mismatch (Recalbox YELLOW)
|
||||
@@ -378,8 +377,6 @@ def compute_severity(
|
||||
return Severity.OK
|
||||
|
||||
|
||||
# ZIP content index
|
||||
|
||||
# Cross-reference: undeclared files used by cores
|
||||
|
||||
|
||||
@@ -416,7 +413,6 @@ def _name_in_index(
|
||||
basename = name.rsplit("/", 1)[-1] if "/" in name else name
|
||||
if basename != name and basename in by_name:
|
||||
return True
|
||||
# Case-insensitive by_name lookup
|
||||
if by_name_lower:
|
||||
key = name.lower()
|
||||
if key in by_name_lower:
|
||||
@@ -508,7 +504,6 @@ def find_undeclared_files(
|
||||
config.get("verification_mode")
|
||||
)
|
||||
|
||||
# Collect data_directory refs
|
||||
declared_dd: set[str] = set()
|
||||
for sys_id, system in config.get("systems", {}).items():
|
||||
for dd in system.get("data_directories", []):
|
||||
@@ -601,7 +596,6 @@ def find_undeclared_files(
|
||||
archive = f.get("archive")
|
||||
seen_files.add(seen_key)
|
||||
|
||||
# Archived files are grouped by archive
|
||||
if archive:
|
||||
archive_key = (
|
||||
archive,
|
||||
@@ -722,7 +716,6 @@ def find_undeclared_files(
|
||||
undeclared = [u for u in undeclared if id(u) not in settled]
|
||||
undeclared.extend(added)
|
||||
|
||||
# Append grouped archive entries
|
||||
for entry in sorted(archive_entries.values(), key=lambda e: e["name"]):
|
||||
undeclared.append(entry)
|
||||
|
||||
@@ -762,7 +755,6 @@ def find_exclusion_notes(
|
||||
|
||||
emu_display = profile.get("emulator", emu_name)
|
||||
|
||||
# Launcher excluded entirely
|
||||
if profile.get("type") == "launcher":
|
||||
notes.append(
|
||||
{
|
||||
@@ -1088,7 +1080,6 @@ def verify_platform(
|
||||
validation_index = _build_validation_index(platform_profiles)
|
||||
validation_owners = destination_owners(platform_profiles)
|
||||
|
||||
# Filter systems by target
|
||||
if not target_cores:
|
||||
plat_cores = None
|
||||
verify_systems = filter_systems_by_target(
|
||||
@@ -1098,9 +1089,7 @@ def verify_platform(
|
||||
platform_cores=plat_cores,
|
||||
)
|
||||
|
||||
# Per-entry results
|
||||
details = []
|
||||
# Per-destination aggregation
|
||||
file_status: dict[str, str] = {}
|
||||
file_required: dict[str, bool] = {}
|
||||
file_severity: dict[str, str] = {}
|
||||
@@ -1340,7 +1329,6 @@ def verify_platform(
|
||||
config, emulators_dir, emu_profiles, target_cores=target_cores
|
||||
)
|
||||
|
||||
# Ground truth coverage
|
||||
gt_filenames = set(validation_index)
|
||||
profiled_names: set[str] = set()
|
||||
for profile in profiles.values():
|
||||
@@ -1630,7 +1618,6 @@ def print_platform_result(
|
||||
ok_count = c[Severity.OK]
|
||||
problems = total - ok_count
|
||||
|
||||
# Summary line
|
||||
if not reads_file_contents(mode):
|
||||
if problems:
|
||||
missing = c.get(Severity.WARNING, 0) + c.get(Severity.CRITICAL, 0)
|
||||
@@ -2289,7 +2276,6 @@ def print_emulator_result(result: dict, verbose: bool = False) -> None:
|
||||
f" Note: data directory '{ref}' required but not included (use refresh_data_dirs.py)"
|
||||
)
|
||||
|
||||
# Ground truth coverage footer
|
||||
gt_cov = result.get("ground_truth_coverage")
|
||||
if gt_cov and gt_cov["total"] > 0:
|
||||
pct = round(gt_cov["with_validation"] * 100 / gt_cov["total"])
|
||||
@@ -2449,7 +2435,6 @@ def _main(held: contextlib.ExitStack) -> None:
|
||||
_run_listing(args, parser)
|
||||
return
|
||||
|
||||
# Mutual exclusion
|
||||
modes = sum(1 for x in (args.platform, args.all, args.emulator, args.system) if x)
|
||||
if modes == 0:
|
||||
parser.error("Specify --platform, --all, --emulator, or --system")
|
||||
|
||||
@@ -6125,5 +6125,33 @@ class SkipPacksHelpSaysWhatRuns(unittest.TestCase):
|
||||
self.assertEqual(skipped, {"4", "4b", "4c", "6"})
|
||||
|
||||
|
||||
class CommentsSayWhy(unittest.TestCase):
|
||||
"""A one-line comment that names the next statement ("# Load registry",
|
||||
"# Build cross-reference indexes") repeats the code. A comment that
|
||||
stays says why (for, only, so, because)."""
|
||||
|
||||
LABEL = re.compile(
|
||||
r"^\s*# (Load|Build|Collect|Determine|Resolve|Group|Copy|Count|Append|"
|
||||
r"Filter|Separate|Compute|Create|Get|Check|Generate|Write|Read|Parse|Sort)"
|
||||
r"\b[^.:;,()]{0,60}$"
|
||||
)
|
||||
REASON = re.compile(r"\b(for|only|so|because|if|when|unless|before|after)\b")
|
||||
|
||||
def test_no_label_comment_restates_the_next_line(self):
|
||||
found = []
|
||||
for name in ("verify.py", "generate_site.py", "generate_truth.py"):
|
||||
lines = (ROOT / "scripts" / name).read_text(encoding="utf-8").splitlines()
|
||||
for index, line in enumerate(lines):
|
||||
if not self.LABEL.match(line) or self.REASON.search(line):
|
||||
continue
|
||||
around = (lines[index - 1].strip(), lines[index + 1].strip())
|
||||
if any(neighbour.startswith("#") for neighbour in around):
|
||||
continue
|
||||
if not line.startswith(" "):
|
||||
continue
|
||||
found.append(f"{name}:{index + 1}: {line.strip()}")
|
||||
self.assertEqual(found, [])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user