diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index de71b2b..b9e07ef 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -19,7 +19,7 @@ You can add, edit or remove entries by opening a pull request. All data is stored in [`awesome-privacy.yml`](https://github.com/Lissy93/awesome-privacy/blob/main/awesome-privacy.yml). -If you're adding, editing or removing a listing - **this is the only file you need to edit**. +If you're adding, editing or removing a listing - **this is the only file you need to edit**. Don't edit the README directly, as this is auto-generated from the YAML file. ### Process @@ -74,12 +74,11 @@ Usually these entries go within the "Notable Mentions" section instead._ Your pull request must follow these requirements. Failure to do so, might result in it being closed. -- Do not edit the README directly when adding / editing a listing +- Do not edit the README directly when adding / editing a listing (it's auto-generated!) - Ensure your PR is not a duplicate, search for existing / previous submissions first -- You must respond to any comments or requests for changes in a timely manner, 48-hours maximum +- You must respond to any comments or requests for changes in a timely manner, 14 days maximum - Write short but descriptive git commit messages, under 50 characters. This must be in the format of `Adds [software-name] to [section-name]`. Your PR will be rejected if you name it `Updates README.md` - Only include a single addition / amendment / removal, per pull request -- If your pull request contains multiple commits, you must squash them first - You must complete each of the sections in the pull request template. Do not delete it! - Where applicable, include links to supporting material for your addition: git repo, docs, recent security audits, etc. This will make researching it much easier for reviewers - While adding new software to the list, don't make your entry read like an advert. Be objective, and include drawbacks as well as strengths @@ -90,8 +89,8 @@ Your pull request must follow these requirements. Failure to do so, might result - You must adhere to the Contributor Covenant Code of Conduct - Don't open a Draft / WIP pull request while you work on the guidelines. A pull request should be 100% ready and should adhere to all the above guidelines when you open it - Your changes must be correctly spelled, and with good grammar -- Your changes must be correctly formatted, in valid markdown -- The addition title must be a link the project, and in bold +- Your changes must be correctly formatted, in valid yaml and markdown +- The addition title must be a link the project - The addition description must be no less than 50, and no more than 250 characters, keep it clear and to the point --- @@ -103,8 +102,6 @@ This file may look a bit daunting to start with, but don't worry - it's pretty s ### Top-Level Structure - - ```mermaid --- title: Class Diagram diff --git a/lib/checks/check-additions.py b/lib/checks/check-additions.py index b637d89..af51e03 100644 --- a/lib/checks/check-additions.py +++ b/lib/checks/check-additions.py @@ -35,6 +35,22 @@ OPENSOURCE_MSG = ( f" [Requirements]({CONTRIBUTING}#requirements)." " Please ensure that this is justified in your PR body." ) +DUPLICATE_NAME_MSG = ( + "A service named `{name}` already exists (in {location})." + " If this is a different service, please clarify in your PR description" +) +DUPLICATE_URL_MSG = ( + "The URL `{url}` is already associated with `{existing}`." + " Please check this isn't a duplicate submission" +) +DESC_LENGTH_MSG = ( + "Description length ({length} chars) is outside the recommended 50\u2013250" + f" character range. Please see our [Contributing Guidelines]({CONTRIBUTING}#description)" +) +OPENSOURCE_GITHUB_MSG = ( + "You marked this service as open source but didn't include a `github` field." + " Please add the repository link" +) def load_json(path): @@ -81,7 +97,7 @@ def check_required_fields(diff, head): for svc in diff.get("services", {}).get("added", []): fields = svc.get("fields", {}) for f in REQUIRED_FIELDS: - if not fields.get(f): + if fields.get(f) is None: missing.add(f) for svc in diff.get("services", {}).get("modified", []): if not head: @@ -92,7 +108,7 @@ def check_required_fields(diff, head): ) if fields: for f in REQUIRED_FIELDS: - if f in changed and not fields.get(f): + if f in changed and fields.get(f) is None: missing.add(f) if missing: names = ", ".join(f"`{f}`" for f in sorted(missing)) @@ -133,6 +149,75 @@ def check_single_entry(diff): return None +def build_name_index(head): + """Build {lowercase_name: "category > section"} from all services.""" + index = {} + if not head: + return index + for cat in head.get("categories", []): + cn = cat.get("name", "") + for sec in cat.get("sections", []): + sn = sec.get("name", "") + for svc in sec.get("services", []): + name = svc.get("name", "").lower().strip() + if name: + index[name] = f"{cn} > {sn}" + return index + + +def build_url_index(head): + """Build {url: service_name} from all services, skipping empty URLs.""" + index = {} + if not head: + return index + for cat in head.get("categories", []): + for sec in cat.get("sections", []): + for svc in sec.get("services", []): + url = svc.get("url", "") + if url: + index[url] = svc.get("name", "") + return index + + +def check_duplicate_name(diff, name_index): + """Return a finding if an added service name already exists in the YAML.""" + for svc in diff.get("services", {}).get("added", []): + name = svc.get("fields", {}).get("name", "").lower().strip() + if name and name in name_index: + return DUPLICATE_NAME_MSG.format( + name=svc["fields"]["name"], location=name_index[name], + ) + return None + + +def check_duplicate_url(diff, url_index): + """Return a finding if an added service URL already exists in the YAML.""" + for svc in diff.get("services", {}).get("added", []): + url = svc.get("fields", {}).get("url", "") + if url and url in url_index: + return DUPLICATE_URL_MSG.format(url=url, existing=url_index[url]) + return None + + +def check_description_length(diff): + """Return a finding if an added service description is outside 50-250 chars.""" + for svc in diff.get("services", {}).get("added", []): + desc = svc.get("fields", {}).get("description", "") + length = len(desc) + if length < 50 or length > 250: + return DESC_LENGTH_MSG.format(length=length) + return None + + +def check_opensource_github(diff): + """Return a finding if an added service is open source but has no github field.""" + for svc in diff.get("services", {}).get("added", []): + fields = svc.get("fields", {}) + if fields.get("openSource") is True and not fields.get("github"): + return OPENSOURCE_GITHUB_MSG + return None + + def main(): findings = [] try: @@ -158,6 +243,25 @@ def main(): finding = check_open_source(diff) if finding: findings.append(finding) + + name_index = build_name_index(head) + url_index = build_url_index(head) + + finding = check_duplicate_name(diff, name_index) + if finding: + findings.append(finding) + + finding = check_duplicate_url(diff, url_index) + if finding: + findings.append(finding) + + finding = check_description_length(diff) + if finding: + findings.append(finding) + + finding = check_opensource_github(diff) + if finding: + findings.append(finding) except Exception: pass diff --git a/lib/checks/check-project.py b/lib/checks/check-project.py index ffa78db..abb2d87 100644 --- a/lib/checks/check-project.py +++ b/lib/checks/check-project.py @@ -126,21 +126,31 @@ def get_services(diff, key): def check_links(diff, head): - """Return LINK_MSG if any service URL is unreachable.""" + """Return LINK_MSG if any service URL or icon URL is unreachable.""" for svc in get_services(diff, "added"): - url = svc.get("fields", {}).get("url") + fields = svc.get("fields", {}) + url = fields.get("url") if url and not check_url(url): return LINK_MSG + icon = fields.get("icon") + if icon and not check_url(icon): + return LINK_MSG for svc in get_services(diff, "modified"): - if "url" not in svc.get("changed_fields", []): + changed = svc.get("changed_fields", []) + if "url" not in changed and "icon" not in changed: continue head_svc = find_service_in_head( head, svc["category"], svc["section"], svc["service"] ) if head_svc: - url = head_svc.get("url") - if url and not check_url(url): - return LINK_MSG + if "url" in changed: + url = head_svc.get("url") + if url and not check_url(url): + return LINK_MSG + if "icon" in changed: + icon = head_svc.get("icon") + if icon and not check_url(icon): + return LINK_MSG return None diff --git a/lib/checks/check-yaml-diff.py b/lib/checks/check-yaml-diff.py index 44a5285..05cafe9 100644 --- a/lib/checks/check-yaml-diff.py +++ b/lib/checks/check-yaml-diff.py @@ -187,16 +187,18 @@ def main(): write_github_output("has_service_changes", str(bool(added or removed or modified)).lower()) write_step_summary(diff_result) - svc_count = len(added) + len(removed) + len(modified) - if svc_count > 1: - print(red(f"Single-entry rule violation: {svc_count} service changes found."), file=sys.stderr) + added_count = len(added) + if added_count > 1: + print(red(f"Single-entry rule violation: {added_count} service additions found."), file=sys.stderr) sys.exit(EXIT_RULE_VIOLATION) - if svc_count == 0 and len(sections) > 1: + if added_count == 0 and len(sections) > 1: print(red(f"Single-entry rule violation: {len(sections)} section changes found."), file=sys.stderr) sys.exit(EXIT_RULE_VIOLATION) - print(green(f"Single-entry rule passed. {svc_count} service, " - f"{len(sections)} section, {len(categories)} category change(s).")) + total = len(added) + len(removed) + len(modified) + print(green(f"Single-entry rule passed. {total} service " + f"({added_count} added), {len(sections)} section, " + f"{len(categories)} category change(s).")) sys.exit(EXIT_PASS)