-
-
Notifications
You must be signed in to change notification settings - Fork 2
Initial step of migration of beeware.org from Lektor to MkDocs #1
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
Conversation
freakboy3742
left a comment
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.
Post generation script looks great! This would work great as-is; a couple of suggestions inline about ways to simplify the code to make long-term maintenance easier.
blog_script/post_generation.py
Outdated
| def validate_url(url): | ||
| """ | ||
| Validates a URL. | ||
|
|
||
| NOTE: CHECK YOUR ENTRY. There are edge cases where this will pass with an invalid entry. | ||
| """ | ||
| try: |
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.
Some type annotations and elaboration on what it does do would be helpful here:
| def validate_url(url): | |
| """ | |
| Validates a URL. | |
| NOTE: CHECK YOUR ENTRY. There are edge cases where this will pass with an invalid entry. | |
| """ | |
| try: | |
| def validate_url(url: str) -> bool: | |
| """Validates a URL. | |
| NOTE: CHECK YOUR ENTRY. This does a simple HEAD request check, but only if | |
| you are connected to the internet. There are edge cases where this will | |
| succeed with an invalid entry. | |
| """ | |
| try: |
blog_script/post_generation.py
Outdated
| return False | ||
| else: | ||
| parsing_validated = False | ||
| while not parsing_validated: |
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.
Why is a loop needed here? Since both branches of the if have returns, this can only run through once AFAICT.
blog_script/post_generation.py
Outdated
| if choice in post_types: | ||
| post_type = post_types[choice] | ||
| else: | ||
| print("Invalid; you must choose a number from the list.") |
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.
This is entirely valid Python; however it might make more sense to write it as a try/except pair (as "invalid" really is the exception):
| if choice in post_types: | |
| post_type = post_types[choice] | |
| else: | |
| print("Invalid; you must choose a number from the list.") | |
| try: | |
| post_type = post_types[choice] | |
| except KeyError: | |
| print("Invalid; you must choose a number from the list.") |
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.
This is no longer necessary; this code was refactored out.
blog_script/post_generation.py
Outdated
| date = datetime.date.today() | ||
| return { | ||
| "title": blog_title, | ||
| "date": date, |
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.
This isn't incorrect; but if we're only using date once, we might as well use it inline.
| date = datetime.date.today() | |
| return { | |
| "title": blog_title, | |
| "date": date, | |
| return { | |
| "title": blog_title, | |
| "date": datetime.date.today(), |
blog_script/post_generation.py
Outdated
| event_name = input("Event name: ") | ||
| event_url = None | ||
| while event_url is None: | ||
| event_url_entry = input("Event URL: ") | ||
| if validate_url(event_url_entry): | ||
| event_url = event_url_entry |
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.
Some blank lines could aid readability here, making it clear that there are logical "blocks" of activity.
| event_name = input("Event name: ") | |
| event_url = None | |
| while event_url is None: | |
| event_url_entry = input("Event URL: ") | |
| if validate_url(event_url_entry): | |
| event_url = event_url_entry | |
| event_name = input("Event name: ") | |
| event_url = None | |
| while event_url is None: | |
| event_url_entry = input("Event URL: ") | |
| if validate_url(event_url_entry): | |
| event_url = event_url_entry |
blog_script/post_generation.py
Outdated
| valid_event_end_date = False | ||
| while not valid_event_end_date: | ||
| try: | ||
| event_end_date_input = input("Event end date (e.g. 2026-01-01; leave blank if same as event start date): ") or event_start_date_input | ||
| event_end_date = datetime.datetime.strptime(event_end_date_input, "%Y-%m-%d").date() | ||
| valid_event_end_date = True | ||
| except ValueError: | ||
| print("Invalid date format. Must be YYYY-DD-MM format.") |
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.
The logic here is almost identical to the logic for start date. it would be worth pulling out input_date() as a method that takes a prompt (and optionally, a default value), so that the usage here is event_end_date = input_date("Event end date ...:", event_start_date)
You could do similar refactoring for input_url(); you could possibly even go as far as input_choice where there's a "select item from dictionary" logic.
| return content | ||
|
|
||
|
|
||
| class NoAliasDumper(yaml.SafeDumper): |
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.
It would be worth adding a comment here to document what "nonstandard" behavior we're encoding here:
- Disabling aliases (YAML's behavior of factoring out any repeated values as constants that are referenced)
- Ensuring multiline-strings are output with
|syntax
blog_script/post_generation.py
Outdated
| Path(Path(__file__).parent.parent / f"docs/en/news/posts/{filename_metadata["date"].year}/{filename_metadata["categories"][0].lower()}").mkdir(parents=True, exist_ok=True) | ||
| return Path(__file__).parent.parent / f"docs/en/news/posts/{filename_metadata["date"].year}/{filename_metadata["categories"][0].lower()}" / filename |
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.
The pathname here is (a) long and (b) shared between these two lines. It would be worth constructing that pathname as a variable (which also gives an opportunity to break the path over multiple lines of code), and then using that variable twice.
| if filename.is_file(): | ||
| print("Post already exists.") | ||
| else: | ||
| content = yaml.dump(metadata, Dumper=NoAliasDumper, sort_keys=False, width=9999) |
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.
Worth documenting inline why the two extra keys are needed.
blog_script/post_generation.py
Outdated
| payload = dedent("""\ | ||
| Add blog post introduction here. | ||
|
|
||
| <!-- more --> | ||
|
|
||
| Add blog post content here.""") |
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.
I find it helpful to indent "dedented" strings as if it were code - that is, at one level deeper than the surrounding statements. That makes the "scope" of the string clearer:
| payload = dedent("""\ | |
| Add blog post introduction here. | |
| <!-- more --> | |
| Add blog post content here.""") | |
| payload = dedent("""\ | |
| Add blog post introduction here. | |
| <!-- more --> | |
| Add blog post content here.""" | |
| ) |
freakboy3742
left a comment
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.
This is awesome! I've pushed the requested content updates, and some tweaks to the macros and blog script, but this is an incredible effort. Thanks so much - can't wait to see this go live!
blog_script/post_generation.py
Outdated
| further_involvement = input("Is the team involved in another way? (y/N): ") or "N" | ||
| if further_involvement in ["N", "n", "no"]: | ||
| break |
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.
This can be simplified a little, and made more robust; if you check further_involvement[0].upper() == "Y"`, you get a boolean that is only true if you write something that starts with "Y" or "y". Enter, or any variant on "no" will return false.
blog_script/post_generation.py
Outdated
| def validate_url(url: str) -> bool: | ||
| """ | ||
| Validates a URL. |
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.
This is a minor style thing where PyCharm's default is different; but I've always thought docstrings should be on the line with the quotes.
| def validate_url(url: str) -> bool: | |
| """ | |
| Validates a URL. | |
| def validate_url(url: str) -> bool: | |
| """Validates a URL. |
blog_script/post_generation.py
Outdated
| # verification when an internet connection is unavailable. | ||
| parsing_url = urlparse(url) | ||
| if parsing_url.scheme in ["http", "https"] and parsing_url.netloc != "": | ||
| print("URL is the correct format, however it has not been validated. Verify it on post creation.") |
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.
There's a couple of line-length issues and minor formatting things that I think will get picked up by a ruff pass.
blog_script/post_generation.py
Outdated
| def generate_file_path(filename_metadata): | ||
| file_path = f"{re.sub(r"[^\w ]", "", filename_metadata["title"]).lower().replace(" ", "-")}.md" | ||
| docs_path = f"docs/en/news/posts/{filename_metadata["date"].year}/{filename_metadata["categories"][0].lower()}" | ||
| Path(Path(__file__).parent.parent / docs_path).mkdir(parents=True, exist_ok=True) |
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.
- There's no need to wrap this in
Pathif it's already a Path object; - Since the full
.parent.parent/docs_pathis what is being re-used, we might as well factor the parent.parent part intodocs_path.
blog_script/post_generation.py
Outdated
|
|
||
|
|
||
| def generate_entry(metadata, payload): | ||
| filename = generate_file_path(metadata) |
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.
This function is only used once, so it's not really worth factoring out.
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.
As a user ergonomics thing - making this script scripts/new_post.py, and putting a __init__.py in the folder means you can run it as python -m scripts.new_post.
docs/macros.py
Outdated
| organizing_introduction = None | ||
| attending_introduction = None | ||
| find_us = None | ||
| for inv in involvement: | ||
| if inv["type"] == "organizing": | ||
| organizing_introduction = dedent(f"""\ | ||
| {attendees(authors, team)} will be organizing [{event.name}]({event.url}), which will happen {event_timeframe}!\n\n | ||
| """) | ||
| elif inv["type"] == "attending" or "attending" not in inv["type"]: | ||
| attending_introduction = dedent(f"""\ | ||
| {attendees(authors, team)} will be attending [{event.name}]({event.url}) {event_timeframe}!\n\n | ||
| """) | ||
| else: | ||
| find_us = f"You can find us throughout {event.name}:\n\n" | ||
|
|
||
| if organizing_introduction: | ||
| content.append(organizing_introduction) | ||
| if attending_introduction: | ||
| content.append(attending_introduction) | ||
| content.append("\n<!-- more -->\n") | ||
| content.append(f"{event.description}\n\n") | ||
| if find_us: | ||
| content.append(find_us) |
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.
I think this block can be simplified a little by pre-computing the list of all involvements, and checking membership of that list.
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.
A couple of ruff/line length issues in this file.
Migration from Lektor to MkDocs
This PR is the initial step of the migration from Lektor to MkDocs. We have opted to land this as a proof of concept, and intend to finalise the rest of the tasks in subsequent PRs.
Following an audit of the existing site, there have been some changes to content structure.
TODOs for @freakboy3742
Known outstanding tasks before we can go live
- Complete set of branding guidelines.
- Update BeeWare logos on branding page to latest.
- Add explicit anchors to any headers being linked from elsewhere to avoid failures when translating headers.
.readthedocs.yaml--build-with-warningspyproject.tomlbeeware-docs-toolsmainbranchtox.ini--build-with-warningswhere necessaryPR Checklist: