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

Document the automated release process #381

Merged
merged 14 commits into from
May 13, 2024

Conversation

felixfontein
Copy link
Contributor

Based on comments of @gotmax23 in the #release-management:ansible.com room.

docs/automated-process.md Outdated Show resolved Hide resolved
@felixfontein
Copy link
Contributor Author

The current preview of the new page can be seen here: https://ansible--381.org.readthedocs.build/projects/ansible-build-data/automated-process/ (link valid as long as PR is open)

@felixfontein
Copy link
Contributor Author

Maybe we should already move the "Create porting guide PR" to after merging the ansible-build-data PR? This will be necessary for the automated porting guide PR workflow that's currently under construction (ansible/ansible-documentation#121).

Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks @felixfontein!

I added a few suggestions.

docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/release-process.md Show resolved Hide resolved
docs/release-process.md Outdated Show resolved Hide resolved
Copy link
Contributor

@oraNod oraNod left a comment

Choose a reason for hiding this comment

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

Few suggestions. Feel free to take them or leave them.

docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
@felixfontein
Copy link
Contributor Author

Are there any more comments on this? Or should I merge the current version?

@gotmax23
Copy link
Contributor

I'll try to give it a look later today.

Copy link
Contributor

@oraNod oraNod left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @felixfontein

docs/release-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
docs/automated-process.md Outdated Show resolved Hide resolved
1. Trigger [the automated workflow](https://github.com/ansible-community/ansible-build-data/actions/workflows/ansible-release.yml) on the **Actions** tab of the repository and specify the release version, such as 11.2.0, and the major version, such as 11.
The process will create a PR in the [`ansible-build-data` repository](https://github.com/ansible-community/ansible-build-data/).
Afterwards it will wait for approval before continuing with uploading the package to PyPI.
All users with commit rights in the `ansible-build-data` repository will be informed with a notification once the approval is needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe only @ansible-community/release-management-wg is notified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do you think so? That group isn't mentioned in the workflow, so I would assume that GH notifies everyone with appropriate rights, which probably is the set of folks with commit rights.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that's how the environment that @anweshadas created is set up. It's set in the repo settings, not the workflow file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anweshadas can you confirm this?

Copy link
Member

@webknjaz webknjaz Apr 15, 2024

Choose a reason for hiding this comment

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

Why do you think so? That group isn't mentioned in the workflow, so I would assume that GH notifies everyone with appropriate rights, which probably is the set of folks with commit rights.

In GitHub Environments, there's protection rules, of which one is "required reviewers". These are listed as GitHub usernames or teams. The maximum is 6 entries. Whoever is listed will be notified, I don't think this will notify accounts that aren't in those teams or lists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any way to verify this (I don't have access to the settings), so can someone with appropriate access please check this and confirm (or say what else it is)?

Copy link
Member

Choose a reason for hiding this comment

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

I don't have enough privileges in this repo to check this :(

Maybe @oraNod does?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, plz excuse the delayed response. Just getting back from PTO...

It looks like @gotmax23 is correct and only folks in the ansible-community/release-management-wg group are notified. Here's a screenshot of the required reviewers section of the environment:

image

On a side note, I'm surprised that @felixfontein and @webknjaz don't have sufficient repo permissions. Should we address that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot! I've updated the paragraph in 81cc678 and added a footnote explaining how this is configured.

Regarding repo permissions: I wouldn't mind having more permissions :) I have no idea who currently has more permissions and why the permissions are as they are...

docs/automated-process.md Show resolved Hide resolved
@felixfontein
Copy link
Contributor Author

In case #390 gets merged first the process can be simplified a bit. (If this one gets merged first, the docs need up be updated in #390.)

@felixfontein
Copy link
Contributor Author

I still don't know how to adjust #381 (comment) since I do not have access to the repository settings. @anweshadas or anyone else with sufficient rights, can you please check out which names and/or groups are listed as "Required reviewers" for the second workflow step, and how exactly the setting looks like (so we can describe it in the documentation - will make it easier to later validate for someone with appropriate access who didn't set this up)?

@felixfontein
Copy link
Contributor Author

If nobody objects, I'm going to merge this soon. The current preview of the new page is here: https://ansible--381.org.readthedocs.build/projects/ansible-build-data/automated-process/

@felixfontein felixfontein merged commit 155d08d into ansible-community:main May 13, 2024
3 checks passed
@felixfontein felixfontein deleted the docs branch May 13, 2024 18:18
@felixfontein
Copy link
Contributor Author

Thanks everyone for reviewing and figuring things out :)

@oraNod
Copy link
Contributor

oraNod commented May 14, 2024

Thanks @felixfontein !

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.

5 participants