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

Updated outdated deployment steps #3266

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

krissetto
Copy link
Contributor

Updated CONTRIBUTING docs so the deployment steps reflect the current GitHub action based workflow

Signed-off-by: Christopher Petito <47751006+krissetto@users.noreply.github.com>
@krissetto krissetto added the kind/chore Refactor, linter, CI, etc label May 27, 2024
@krissetto krissetto self-assigned this May 27, 2024
* Make sure a release note for the changes included since the last published version is present in [docs/change-log.md](./docs/change-log.md);
* Go to the [`Release`](https://github.com/docker/docker-py/actions/workflows/release.yml) GitHub action page and click on `Run workflow`. Here:
* Insert the version number you want to give to the new package, **without** the `v.` prefix (eg. `7.1.0`). This version should correspond with the one written in [docs/change-log.md](./docs/change-log.md);
* Make sure `Dry run` is checked;
Copy link
Member

Choose a reason for hiding this comment

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

Not sure we need to document the dry-run option, or ossify it as part of the actual release process, as it's just a debugging/testing tool. At most, I'd explain what it does i.e. "If dry-run is checked, then the package is build but a release is not created/uploaded to pypi"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my logic here was "better safe than sorry" (considering past events), but i'm ok with changing this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It removes "if-branching" from the steps and can be followed as-is in a sense

Copy link
Member

Choose a reason for hiding this comment

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

to clarify my point, I just mean that running a dry-run first is not a necessary step of the release process, so I don't think we should document it as such, since it might introduce confusion "do I have to do a dry run before releasing?". Instead we could just explain what it does (and if we want, mention something like "it's advisable to test the release pipeline before release by checking the "dry-run" option).

Copy link
Contributor Author

@krissetto krissetto May 27, 2024

Choose a reason for hiding this comment

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

Yeah I get your point. I agree that technically speaking a dry run is not necessary, but I'd argue it should be part of the "release process" so that people double check the version number, and only those in-the-know (or confident enough) should skip it at their discretion.

I'm saying this mostly because the result of the pipeline is yes a "draft release" on github, but at that point of the process the package has already been published to pypi and users might be downloading it before we realize and correct any potential issue.

No super hard feelings here, lets see if others have also differing opinions on this

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 disagree, I just think that including "dry-run" in that section removes clarity about what it actually does. The same can be achieved by separately explaining what "dry-run" does, and strongly advising it's use.

@thaJeztah ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/chore Refactor, linter, CI, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants