Skip to content
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

Independent generation of latest.json file #50

Closed
wants to merge 0 commits into from

Conversation

dgw
Copy link
Member

@dgw dgw commented Jun 14, 2024

The script loads the most recent releases from PyPI now, instead of being part of the changelog script (since the changelog won't necessarily include dev builds, prereleases, etc.). The unstable version is no longer just an alias to the current stable version if there is a newer prerelease.

Leans on packaging.version to do the heavy lifting (parsing version specifiers into sortable Version objects). Fortunately we already get packaging and requests for free when installing _sopel to document it and its plugins.

I expect the first PR build to fail. It's an old branch from before #48 and there are some things I need to fixup anyway.

@dgw dgw added the build label Jun 14, 2024
@dgw dgw force-pushed the independent-latest.json branch from 9d350d6 to 8ff1379 Compare June 14, 2024 20:13
@dgw
Copy link
Member Author

dgw commented Jun 14, 2024

Indeed, the first PR build did fail. But I have performed the fixups I needed to, rebased the branch, and now it works.

To illustrate the patch a bit better, here is the old latest.json as of

{
    "version": "8.0.0",
    "unstable": "8.0.0",
    "release_notes": "https://sopel.chat/changelog/8.0.0/"
}

And with this patch, that turns into:

{
    "version": "8.0.0",
    "unstable": "8.0.0",
    "release_notes": "https://sopel.chat/changelog/8.0.0/",
    "unstable_notes": "https://github.com/sopel-irc/sopel/releases/v8.0.0"
}

The unstable_notes item now exists, something the previous code didn't bother doing. It points to the relevant release tag on GitHub since prerelease versions don't appear in the NEWS file (and the site's /changelog/ section therefore won't have them).

@dgw dgw marked this pull request as ready for review June 14, 2024 20:20
Copy link
Contributor

@SnoopJ SnoopJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As expected I had a lot to say about this but none of it is a show-stopper. LGTM

generate_latest_json.py Outdated Show resolved Hide resolved
generate_latest_json.py Outdated Show resolved Hide resolved
generate_latest_json.py Outdated Show resolved Hide resolved
generate_latest_json.py Outdated Show resolved Hide resolved
generate_latest_json.py Outdated Show resolved Hide resolved
generate_latest_json.py Outdated Show resolved Hide resolved
generate_latest_json.py Outdated Show resolved Hide resolved
@dgw
Copy link
Member Author

dgw commented Jun 28, 2024

One of the suggestions—the one to avoid break—ironically broke the control flow, but that should be fixed. I'll request a re-review now and head off into the hot STL evening for the night's Activities™. 🌆

The only downside to using json.dump() is that the output is now all one line (but I'm sure I can tweak that if I check the stdlib docs later):

{"version": "8.0.0", "unstable": "8.0.0", "release_notes": "https://sopel.chat/changelog/8.0.0/", "unstable_notes": "https://github.com/sopel-irc/sopel/releases/v8.0.0"}

@dgw dgw requested a review from SnoopJ June 28, 2024 22:21
@SnoopJ
Copy link
Contributor

SnoopJ commented Jun 29, 2024

The only downside to using json.dump() is that the output is now all one line (but I'm sure I can tweak that if I check the stdlib docs later):

Passing indent will take care of that, I usually do indent=4 but any non-None value should work

dgw added a commit that referenced this pull request Jun 29, 2024
Independent generation of `latest.json` file
dgw added a commit that referenced this pull request Jun 29, 2024
Meant to be done as part of #50, but I *might* have forgotten to do that
before hitting Merge. Oh well!
@dgw dgw closed this Jun 29, 2024
@dgw dgw force-pushed the independent-latest.json branch from 8b595b9 to 81876cf Compare June 29, 2024 05:04
@dgw dgw deleted the independent-latest.json branch June 29, 2024 05:05
@dgw
Copy link
Member Author

dgw commented Jun 29, 2024

Hmm, I did my pushes in the wrong order and confused the heck out of GitHub. All of the feedback here was folded into a squashed commit, 81876cf, and then merged to master via cf6d998, but I did the merge "locally" (in Gitpod) and pushed master first. GH thinks the PR has no content, instead of detecting that its HEAD was merged. Oops. ☹️

Thanks for the review @SnoopJ (and @Exirel, the silent partner on IRC)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants