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

Reimplement bump-readme.sh in Go #304

Merged
merged 2 commits into from
Mar 5, 2025
Merged

Reimplement bump-readme.sh in Go #304

merged 2 commits into from
Mar 5, 2025

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Mar 4, 2025

This replaces the core implementation of bump-readme.sh with Go.

The new Go tool takes a versions.json input that has a list of release
versions and dates, as well as taking the README.rst on stdin, and it
substitutes the versions with the latest versions from the JSON file,
sending the output to stdout.

The bump-readme.sh script is now reduced in scope to remove most sed
usage, it now:

  • Pulls the latest release versions & dates from the GitHub API
  • Pushes the latest stable release version into stable.txt and
    Documentation/_static/stable-version.json
  • Runs the Go implementation to substitute the versions correctly
  • Checks the table formatting to complain if it is misaligned

Overall this should make the implementation more reliable in replacing
the relevant version / date fields, regardless of what the latest
versions are. It also significantly reduces the usage of sed, which
should generally bring this scripting closer to cross-platform
friendliness. Finally with more of the implementation in Go, if
something is wrong, the community is more likely to be able to dig into
why and propose a fix.

In future we can probably push this into the main release binary as a
step six so we can automate this piece in GHA, but for now I just
implemented it separately as an incremental step in the right direction.

This is primarily just moving the existing functionality within
functions so that the script is easier to read, and so that when a
subsequent upcoming change deletes portions of the functionality, the
diff is easier to read. Might break something, didn't test this
intermediate form.

Signed-off-by: Joe Stringer <[email protected]>
@joestringer

This comment was marked as resolved.

@joestringer joestringer requested a review from aanm March 4, 2025 01:59
@joestringer joestringer marked this pull request as ready for review March 4, 2025 01:59
@joestringer joestringer force-pushed the pr/joe/bump-readme branch 5 times, most recently from 5255fe3 to ab0e73c Compare March 4, 2025 18:33
This replaces the core implementation of bump-readme.sh with Go.

The new Go tool takes a versions.json input that has a list of release
versions and dates, as well as taking the README.rst on stdin, and it
substitutes the versions with the latest versions from the JSON file,
sending the output to stdout.

The bump-readme.sh script is now reduced in scope to remove most sed
usage, it now:
- Pulls the latest release versions & dates from the GitHub API
- Pushes the latest stable release version into stable.txt and
  Documentation/_static/stable-version.json
- Runs the Go implementation to substitute the versions correctly
- Checks the table formatting to complain if it is misaligned

Overall this should make the implementation more reliable in replacing
the relevant version / date fields, regardless of what the latest
versions are. It also significantly reduces the usage of sed, which
should generally bring this scripting closer to cross-platform
friendliness. Finally with more of the implementation in Go, if
something is wrong, the community is more likely to be able to dig into
why and propose a fix.

Signed-off-by: Joe Stringer <[email protected]>
Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

@joestringer do you think we could make this automated as part of step 4? 🤔

@joestringer
Copy link
Member Author

I like that idea yeah 👍 I'd have to integrate that as part of an upcoming release (to avoid breaking the release process for other release managers during patch release prep)

@joestringer
Copy link
Member Author

Filed #305 to follow up on that idea.

@joestringer joestringer merged commit cb7e519 into main Mar 5, 2025
1 check passed
@joestringer joestringer deleted the pr/joe/bump-readme branch March 5, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants