From acf8a6a64c67f57457a6fd74f99adb1fecaba20b Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Tue, 23 Nov 2021 17:49:42 +0530 Subject: [PATCH] Address reviews --- news/0.2.0.md | 6 + news/next/.gitkeep | 0 news/next/README.md | 1 + scripts/news/__init__.py | 2 - scripts/news/__main__.py | 138 +++---------------- scripts/news/check_news_workflow/__main__.py | 25 +--- scripts/news/config.toml | 9 -- scripts/news/constants.py | 38 +++++ scripts/news/template.md.jinja | 2 +- scripts/news/utils.py | 110 ++++++++++----- tox.ini | 2 +- 11 files changed, 144 insertions(+), 189 deletions(-) create mode 100644 news/0.2.0.md create mode 100644 news/next/.gitkeep create mode 100644 news/next/README.md delete mode 100644 scripts/news/config.toml create mode 100644 scripts/news/constants.py diff --git a/news/0.2.0.md b/news/0.2.0.md new file mode 100644 index 00000000..b555b233 --- /dev/null +++ b/news/0.2.0.md @@ -0,0 +1,6 @@ +# Modmail 0.2.0 (2021-11-23) + +--- + +### Features +- qwerty (#89) diff --git a/news/next/.gitkeep b/news/next/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/news/next/README.md b/news/next/README.md new file mode 100644 index 00000000..cc7c1ca7 --- /dev/null +++ b/news/next/README.md @@ -0,0 +1 @@ +All your changelogs for upcoming release! diff --git a/scripts/news/__init__.py b/scripts/news/__init__.py index 69a40730..f102a9ca 100644 --- a/scripts/news/__init__.py +++ b/scripts/news/__init__.py @@ -1,3 +1 @@ -ERROR_MSG_PREFIX = "Oh no! 💥 💔 💥" - __version__ = "0.0.1" diff --git a/scripts/news/__main__.py b/scripts/news/__main__.py index 4d86ab85..c0693971 100644 --- a/scripts/news/__main__.py +++ b/scripts/news/__main__.py @@ -1,104 +1,18 @@ -import os -import subprocess from collections import defaultdict from datetime import datetime, timezone -from pathlib import Path from typing import Optional import click -import requests - -from . import ERROR_MSG_PREFIX, __version__ -from .utils import ( - NotRequiredIf, - err, - get_metadata_from_file, - get_project_meta, - glob_fragments, - load_toml_config, - nonceify, - out, - render_fragments, -) - - -PR_ENDPOINT = "https://api.github.com/repos/discord-modmail/modmail/pulls/{number}" -BAD_RESPONSE = { - 404: "Pull request not located! Please enter a valid number!", - 403: "Rate limit has been hit! Please try again later!", -} - -TEMPLATE = """ -# Please write your news content. When finished, save the file. -# In order to abort, exit without saving. -# Lines starting with \"#\" are ignored. - -""".lstrip() -NO_NEWS_PATH_ERROR = ( - f"{ERROR_MSG_PREFIX} `news/next/` doesn't exist.\nYou are either in the wrong directory while" - " running this command (should be in the project root) or the path doesn't exist, if it " - "doesn't exist please create it and run this command again :) Happy change-logging!" -) -CONFIG = load_toml_config() -SECTIONS = [_type for _type, _ in CONFIG.get("types").items()] - - -def save_news_fragment(ctx: click.Context, gh_pr: int, nonce: str, news_entry: str, news_type: str) -> None: - """Save received changelog data to a news file.""" - date = datetime.now(timezone.utc).strftime("%Y-%m-%d") - path = Path(Path.cwd(), f"news/next/{news_type}/{date}.pr-{gh_pr}.{nonce}.md") - if not path.parents[1].exists(): - err(NO_NEWS_PATH_ERROR, fg="blue") - ctx.exit(1) - elif not path.parents[0].exists(): - make_news_type_dir = click.confirm( - f"Should I make the new type DIR for the news type at {path.parents[0]}" - ) - if make_news_type_dir: - path.parents[0].mkdir(exist_ok=True) - elif path.exists(): - # The file exists - err(f"{ERROR_MSG_PREFIX} {Path(os.path.relpath(path, start=Path.cwd()))} already exists") - ctx.exit(1) - - text = str(news_entry) - with open(path, "wt", encoding="utf-8") as file: - file.write(text) - - # Add news fragment to git stage - subprocess.run(["git", "add", "--force", path]).check_returncode() - - out( - f"All done! ✨ 🍰 ✨ Created news fragment at {Path(os.path.relpath(path, start=Path.cwd()))}" - "\nYou are now ready for commit!" - ) - - -def validate_pull_request_number( - ctx: click.Context, _param: click.Parameter, value: Optional[int] -) -> Optional[int]: - """Check if the given pull request number exists on the github repository.""" - r = requests.get(PR_ENDPOINT.format(number=value)) - if r.status_code == 403: - if r.headers.get("X-RateLimit-Remaining") == "0": - err(f"{ERROR_MSG_PREFIX} Ratelimit reached, please retry in a few minutes.") - ctx.exit() - err(f"{ERROR_MSG_PREFIX} Cannot access pull request.") - ctx.exit() - elif r.status_code in (404, 410): - err(f"{ERROR_MSG_PREFIX} PR not found.") - ctx.exit() - elif r.status_code != 200: - err(f"{ERROR_MSG_PREFIX} Error while fetching issue, retry again after sometime.") - ctx.exit() - - return value +from . import __version__ +from .constants import * +from .utils import * @click.group(context_settings=dict(help_option_names=["-h", "--help"]), invoke_without_command=True) @click.version_option(version=__version__) -def cli_main() -> None: +@click.pass_context +def cli_main(ctx: click.Context) -> None: """ Modmail News 📜🤖. @@ -107,7 +21,9 @@ def cli_main() -> None: contributors and maintainers to work with news files (changelogs) by automating the process of generating, compiling and validating them! """ - ... + if not ctx.args and not ctx.resilient_parsing and not ctx.command: + click.echo(ctx.get_help()) + ctx.exit() @cli_main.command("add") @@ -131,24 +47,19 @@ def cli_main() -> None: prompt=True, ) @click.option( - "--pr-number", + "--pr", type=int, prompt=True, callback=validate_pull_request_number, ) @click.pass_context -def cli_add_news(ctx: click.Context, message: str, editor: str, type: str, pr_number: int) -> None: +def cli_add_news(ctx: click.Context, message: str, editor: str, type: str, pr: int) -> None: """Add a news entry 📜 to the current discord-modmail repo for your awesome change!""" if not message: message_notes = [] while True: content = click.edit( - ( - "# Please write your news content. When finished, save the file.\n" - "# In order to abort, exit without saving.\n" - "# Lines starting with '#' are ignored.\n" - "\n".join(message_notes) - ), + "\n".join((TEMPLATE, *message_notes)), editor=editor, extension="md", ) @@ -167,7 +78,7 @@ def cli_add_news(ctx: click.Context, message: str, editor: str, type: str, pr_nu break - save_news_fragment(ctx, pr_number, nonceify(message), message, type) + save_news_fragment(ctx, pr, nonceify(message), message, type) @cli_main.command("build") @@ -192,7 +103,7 @@ def cli_build_news(ctx: click.Context, edit: Optional[bool], keep: bool) -> None for filename in filenames: if not filename.endswith(".md"): continue - _file_metadata[filename] = get_metadata_from_file(Path(filename)) + _file_metadata[filename] = get_metadata_from_news(Path(filename)) # Group metadata according to news_type for path, fragment in _file_metadata.items(): @@ -201,28 +112,16 @@ def cli_build_news(ctx: click.Context, edit: Optional[bool], keep: bool) -> None fragment["path"] = path file_metadata[news_type].append(fragment) - template = CONFIG["core"].get("template") - if not template: - template = Path(Path.cwd(), "scripts/news/template.md.jinja") - else: - template = Path(Path.cwd(), f"scripts/news/{template}") - - if not template.exists(): - err( - f"{ERROR_MSG_PREFIX} Template at {template.relative_to(Path.cwd())} not found :(. Make sure " - f"your path is relative to `scripts/news`!" - ) - name, version = get_project_meta() version_news = render_fragments( - section_names=CONFIG["types"], - template=template, + sections=SECTIONS, + template=TEMPLATE_FILE_PATH, metadata=file_metadata, wrap=True, version_data=(name, version), date=date, ) - news_path = Path(Path.cwd(), f"news/{version}.md") + news_path = Path(REPO_ROOT, f"news/{version}.md") with open(news_path, mode="w") as file: file.write(version_news) @@ -233,10 +132,9 @@ def cli_build_news(ctx: click.Context, edit: Optional[bool], keep: bool) -> None click.edit(filename=str(news_path)) if not keep: - files = Path(Path.cwd(), "scripts/news/next") - for news_fragment in files.glob("*.md"): + for news_fragment in NEWS_NEXT.glob("*/*.md"): os.remove(news_fragment) - out("🍰 Cleared existing `scripts/news/next` news fragments!") + out("🍰 Cleared existing `news/next` news fragments!") if __name__ == "__main__": diff --git a/scripts/news/check_news_workflow/__main__.py b/scripts/news/check_news_workflow/__main__.py index 7ff6be5c..0d56934a 100644 --- a/scripts/news/check_news_workflow/__main__.py +++ b/scripts/news/check_news_workflow/__main__.py @@ -1,35 +1,16 @@ import pathlib import re import sys -import traceback from typing import Tuple import requests -import tomli NEWS_NEXT_DIR = "news/next/" SKIP_NEWS_LABEL = "skip changelog" GH_API_URL = "https://api.github.com/" HEADERS = {"accept": "application/vnd.github.v3+json"} - - -def load_toml_config() -> dict: - """Load the news TOML configuration file and exit if found to be invalid.""" - config_path = pathlib.Path(pathlib.Path.cwd(), "scripts/news/config.toml") - - try: - with open(config_path, mode="r") as file: - toml_dict = tomli.loads(file.read()) - except tomli.TOMLDecodeError as e: - message = "Invalid news configuration at {0}\n{1}".format( - config_path, - "".join(traceback.format_exception_only(type(e), e)), - ) - print(message) - sys.exit(1) - else: - return toml_dict +REPO_ORG_NAME = "discord-modmail/modmail" FILENAME_RE = re.compile( @@ -48,7 +29,7 @@ def is_news_dir(filename: str) -> bool: def main(pr: int) -> Tuple[str, bool]: """Main function to check for a changelog entry.""" - r = requests.get(f"{GH_API_URL}repos/discord-modmail/modmail/pulls/{pr}/files", headers=HEADERS) + r = requests.get(f"{GH_API_URL}repos/{REPO_ORG_NAME}/pulls/{pr}/files", headers=HEADERS) files_changed = r.json() in_next_dir = file_found = False @@ -64,7 +45,7 @@ def main(pr: int) -> Tuple[str, bool]: status = (f"News entry found in {NEWS_NEXT_DIR}", True) break else: - _r = requests.get(f"{GH_API_URL}repos/discord-modmail/modmail/pulls/{pr}", headers=HEADERS) + _r = requests.get(f"{GH_API_URL}repos/{REPO_ORG_NAME}/pulls/{pr}", headers=HEADERS) pr_data = _r.json() labels = [label["name"] for label in pr_data["labels"]] if SKIP_NEWS_LABEL in labels: diff --git a/scripts/news/config.toml b/scripts/news/config.toml deleted file mode 100644 index ebb543ec..00000000 --- a/scripts/news/config.toml +++ /dev/null @@ -1,9 +0,0 @@ -[types] -feature = { name = "Features" } -trivial = { name = "Trivial/Internal Changes" } -improvement = { name = "Improvements" } -bugfix = { name = "Bug Fixes" } -doc = { name = "Improved Documentation" } -deprecation = { name = "Deprecations" } -breaking = { name = "Breaking Changes" } -internal = { name = "Internal" } diff --git a/scripts/news/constants.py b/scripts/news/constants.py new file mode 100644 index 00000000..e8342cc2 --- /dev/null +++ b/scripts/news/constants.py @@ -0,0 +1,38 @@ +import os +from pathlib import Path + +import modmail + + +PR_ENDPOINT = "https://api.github.com/repos/discord-modmail/modmail/pulls/{number}" +BAD_RESPONSE = { + 404: "Pull request not located! Please enter a valid number!", + 403: "Rate limit has been hit! Please try again later!", +} + +TEMPLATE_FILE_PATH = Path(Path(__file__).parent, "template.md.jinja") +REPO_ROOT = Path(modmail.__file__).parent.parent +NEWS_NEXT = Path(REPO_ROOT, "news/next") + +ERROR_MSG_PREFIX = "Oh no! 💥 💔 💥" +TEMPLATE = """ +# Please write your news content. When finished, save the file. +# In order to abort, exit without saving. +# Lines starting with "#" are ignored. + +""".lstrip() +NO_NEWS_PATH_ERROR = ( + f"{ERROR_MSG_PREFIX} {Path(os.path.relpath(NEWS_NEXT, start=Path.cwd()))} doesn't exist, please create it" + f" and run this command again :) Happy change-logging!" +) + +SECTIONS = { + "feature": "Features", + "trivial": "Trivial/Internal Changes", + "improvement": "Improvements", + "bugfix": "Bug Fixes", + "doc": "Improved Documentation", + "deprecation": "Deprecations", + "breaking": "Breaking Changes", + "internal": "Internal", +} diff --git a/scripts/news/template.md.jinja b/scripts/news/template.md.jinja index c5be05d2..feab6991 100644 --- a/scripts/news/template.md.jinja +++ b/scripts/news/template.md.jinja @@ -9,7 +9,7 @@ --- {% for section, fragments in metadata.items() %} -### {{ section_names[section] }} +### {{ sections[section] }} {% for fragment in fragments %} - {{ fragment["news_entry"].strip("\n") }} (#{{ fragment["gh_pr"].split("-")[1] }}) {% endfor %} diff --git a/scripts/news/utils.py b/scripts/news/utils.py index d4849d43..4508f0da 100644 --- a/scripts/news/utils.py +++ b/scripts/news/utils.py @@ -2,18 +2,30 @@ import datetime import glob import hashlib -import os -import sys import textwrap -import traceback -from pathlib import Path from typing import Any, Dict, List, Mapping, Optional, Tuple, Union +import click +import requests import tomli from click import Context, Option, echo, style from jinja2 import Template -from . import ERROR_MSG_PREFIX +from .constants import * + + +__all__ = ( + "NotRequiredIf", + "get_metadata_from_news", + "get_project_meta", + "glob_fragments", + "err", + "nonceify", + "out", + "render_fragments", + "save_news_fragment", + "validate_pull_request_number", +) def nonceify(body: str) -> str: @@ -64,7 +76,7 @@ def __init__(self, *args, **kwargs): kwargs.get("help", "") + " NOTE: This argument is mutually exclusive with %s" % self.not_required_if ).strip() - super(NotRequiredIf, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) def handle_parse_result( self, ctx: Context, opts: Mapping[str, Any], args: List[str] @@ -84,7 +96,7 @@ def handle_parse_result( else: self.prompt = None - return super(NotRequiredIf, self).handle_parse_result(ctx, opts, args) + return super().handle_parse_result(ctx, opts, args) def sanitize_section(section: str) -> str: @@ -92,12 +104,12 @@ def sanitize_section(section: str) -> str: return section.replace("/", "-").lower() -def glob_fragments(version: str, sections: List[str]) -> List[str]: +def glob_fragments(version: str, sections: Dict[str, str]) -> List[str]: """Glob all news fragments present on the repo.""" filenames = [] base = os.path.join("news", version) - if version != "next": + if version.lower() != "next": wildcard = base + ".md" filenames.extend(glob.glob(wildcard)) else: @@ -105,18 +117,17 @@ def glob_fragments(version: str, sections: List[str]) -> List[str]: wildcard = os.path.join(base, sanitize_section(section), "*.md") entries = glob.glob(wildcard) entries.sort(reverse=True) - deletables = [x for x in entries if x.endswith("/README.md")] - for filename in deletables: - entries.remove(filename) + entries = [x for x in entries if not x.endswith("/README.md")] filenames.extend(entries) return filenames -def get_metadata_from_file(path: Path) -> dict: +def get_metadata_from_news(path: Path) -> dict: """Get metadata information from a news entry.""" new_fragment_file = path.stem - date, gh_pr, news_type, nonce = new_fragment_file.split(".") + date, gh_pr, nonce = new_fragment_file.split(".") + news_type = path.parent.name with open(path, "r", encoding="utf-8") as file: news_entry = file.read() @@ -140,26 +151,8 @@ def get_project_meta() -> Tuple[str, str]: return name, version -def load_toml_config() -> Dict[str, Any]: - """Load the news TOML configuration file and exit if found to be invalid.""" - config_path = Path(Path.cwd(), "scripts/news/config.toml") - - try: - with open(config_path, mode="r") as file: - toml_dict = tomli.loads(file.read()) - except tomli.TOMLDecodeError as e: - message = "Invalid news configuration at {0}\n{1}".format( - config_path, - "".join(traceback.format_exception_only(type(e), e)), - ) - err(message) - sys.exit(1) - else: - return toml_dict - - def render_fragments( - section_names: Dict[str, dict], + sections: Dict[str, str], template: Path, metadata: Dict[str, list], wrap: bool, @@ -173,7 +166,7 @@ def render_fragments( version_data = {"name": version_data[0], "version": version_data[1], "date": date} res = jinja_template.render( - section_names={_type: config["name"] for _type, config in section_names.items()}, + sections=sections.copy(), version_data=version_data, metadata=metadata, ) @@ -194,3 +187,52 @@ def render_fragments( done.append(line) return "\n".join(done).rstrip() + "\n" + + +def save_news_fragment(ctx: click.Context, gh_pr: int, nonce: str, news_entry: str, news_type: str) -> None: + """Save received changelog data to a news file.""" + date = datetime.now(datetime.timezone.utc).strftime("%Y-%m-%d") + path = Path(REPO_ROOT, f"news/next/{news_type}/{date}.pr-{gh_pr}.{nonce}.md") + if not path.parents[1].exists(): + err(NO_NEWS_PATH_ERROR, fg="blue") + ctx.exit(1) + elif not path.parents[0].exists(): + make_news_type_dir = click.confirm( + f"Should I make the new type DIR for the news type at {path.parents[0]}" + ) + if make_news_type_dir: + path.parents[0].mkdir(exist_ok=True) + elif path.exists(): + # The file exists + err(f"{ERROR_MSG_PREFIX} {Path(os.path.relpath(path, start=Path.cwd()))} already exists") + ctx.exit(1) + + text = str(news_entry) + with open(path, "wt", encoding="utf-8") as file: + file.write(text) + + out( + f"All done! ✨ 🍰 ✨ Created news fragment at {Path(os.path.relpath(path, start=Path.cwd()))}" + "\nYou are now ready for commit the changelog!" + ) + + +def validate_pull_request_number( + ctx: click.Context, _param: click.Parameter, value: Optional[int] +) -> Optional[int]: + """Check if the given pull request number exists on the github repository.""" + r = requests.get(PR_ENDPOINT.format(number=value)) + if r.status_code == 403: + if r.headers.get("X-RateLimit-Remaining") == "0": + err(f"{ERROR_MSG_PREFIX} Ratelimit reached, please retry in a few minutes.") + ctx.exit() + err(f"{ERROR_MSG_PREFIX} Cannot access pull request.") + ctx.exit() + elif r.status_code in (404, 410): + err(f"{ERROR_MSG_PREFIX} PR not found.") + ctx.exit() + elif r.status_code != 200: + err(f"{ERROR_MSG_PREFIX} Error while fetching issue, retry again after sometime.") + ctx.exit() + + return value diff --git a/tox.ini b/tox.ini index 87115ef2..70c6a70b 100644 --- a/tox.ini +++ b/tox.ini @@ -28,7 +28,7 @@ ignore= per-file-ignores= tests/*:,ANN,S101,F401 docs.py:B008 - scripts/news/*:S404,S603,S607 + scripts/news/*:F405,F403 [isort] profile=black