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

hack: extract release script to generate download info from the release doc #187

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

obnoxxx
Copy link
Collaborator

@obnoxxx obnoxxx commented Nov 27, 2024

This extracts the script added to the the release process document as in PR #185 from the document.

This script can be invoked to create the downloads section for the release notes for a given release.

@obnoxxx
Copy link
Collaborator Author

obnoxxx commented Nov 27, 2024

I updated the script to fix a shellcheck error caught by the ci.

Now this is not a verbatim extraction from the doc any more.

hack/release-gen-download-section.sh Outdated Show resolved Hide resolved
@phlogistonjohn
Copy link
Collaborator

Sorry @obnoxxx but I didn't really mean to make this a fully fledged script. It was more meant as a template someone running the release can use.

@obnoxxx obnoxxx changed the title hack: add release script to generate download info from the release doc hack: extract release script to generate download info from the release doc Dec 5, 2024
@obnoxxx obnoxxx force-pushed the extract-rel-script branch from a6cbf02 to 235f3cf Compare December 5, 2024 09:14
@obnoxxx
Copy link
Collaborator Author

obnoxxx commented Dec 5, 2024

@phlogistonjohn wrote:

Sorry @obnoxxx but I didn't really mean to make this a fully fledged script. It was more meant as a template someone running the release can use.

Understood, but I thought that having to copy and paste from the doc lacks a level of convenience that providing an actual script offers.

@obnoxxx obnoxxx requested a review from anoopcs9 December 5, 2024 13:32
@obnoxxx
Copy link
Collaborator Author

obnoxxx commented Jan 7, 2025

@phlogistonjohn wrote:

Sorry @obnoxxx but I didn't really mean to make this a fully fledged script. It was more meant as a template someone running the release can use.

I still think that an actual script in the repo adds more real convenience than a copy-and-paste template which is better than no automation but clumsy to use at best (IMHO).

since you are opposed to having this, @phlogistonjohn), should I just go ahead and close this PR?

@phlogistonjohn
Copy link
Collaborator

I'm less opposed to it and more 🤷 on having it. if you can get someone else to +1 having it as a script I will be willing to accept it as a separate script. I was also not paying attention to this over the holidays... so don't feel my earlier silence was opposition, just neglect.

@obnoxxx obnoxxx force-pushed the extract-rel-script branch from 235f3cf to 8e710d8 Compare January 7, 2025 18:22
@obnoxxx
Copy link
Collaborator Author

obnoxxx commented Jan 7, 2025

@anoopcs9, github says that you requested changes but I can't see any more what it was that you requested.
Kindly asing you to re-review.

@anoopcs9
Copy link
Collaborator

anoopcs9 commented Jan 8, 2025

@anoopcs9, github says that you requested changes but I can't see any more what it was that you requested.

Because my previous suggestion was resolved without making any changes in the updated version. I've "unresolved" it for now. Let me know what you think.

This extracts a script from the release process document.

This script can be invoked to create the downloads
section for the release notes for a given release.

Signed-off-by: Michael Adam <[email protected]>
@obnoxxx obnoxxx force-pushed the extract-rel-script branch from 3a149ba to 9562237 Compare January 8, 2025 09:36
@obnoxxx
Copy link
Collaborator Author

obnoxxx commented Jan 8, 2025

@anoopcs9 wrote:

@anoopcs9, github says that you requested changes but I can't see any more what it was that you requested.

Because my previous suggestion was resolved without making any changes in the updated version. I've "unresolved" it for now. Let me know what you think.

Thanks for clarifying!

I have now applied and squashed your suggestion.

Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

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

lgtm, thanks.

@mergify mergify bot merged commit d7ea7a9 into samba-in-kubernetes:master Jan 8, 2025
41 checks passed
@anoopcs9
Copy link
Collaborator

anoopcs9 commented Jan 8, 2025

@phlogistonjohn @obnoxxx
Sorry, I forgot that this had a priority-review label which then requires just 1 approval for merge.

@obnoxxx obnoxxx deleted the extract-rel-script branch January 8, 2025 11:07
@phlogistonjohn
Copy link
Collaborator

It's fine. Like I said earlier: I didn't think making a full script was worth the effort... but I am ok with it if enough others wanted it.

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

Successfully merging this pull request may close these issues.

3 participants