Setup release process and other improvements #4

lampajr · 2024-03-21T15:54:57Z

Fixes #3

Changes proposed

Setup release process and doc
Improve documentation with examples and references
Simplify workflows
Set authors / maintainers
Remove openapi from committed files
Remove pull request generation as no more required, we just need to trigger build/test checks
Setup unit tests to be run as part of the CI (this is mainly to check the generated client exists)
Setup nox for multi environments tests automation
Added automated backport

Check List (Check all the applicable boxes)

My code follows the code style of this project.
My change requires changes to the documentation.
I have updated the documentation accordingly.
All new and existing tests passed.

lampajr · 2024-03-25T10:11:27Z

Hey @stalep , with this PR I implemented a further simplication to the repository:

By removing the openapi from the committed files, we no longer need to create a pull request with those changes whenever the openapi chanegs on Horreum, instead we just need to trigger the ci (that will download the proper openapi from Horreum) and check that the build/test work
- We just need to configure Horreum to send some notification to this repo everytime we change something on our master or stable branches
- Unfortunately with python there is no real guarantee that it would work because there is no compilation, therefore I think we should implement some tests that check ~everything is working
Other improvements are summarized in the pr description

pyproject.toml

stalep · 2024-03-25T10:27:48Z

Hey @stalep , with this PR I implemented a further simplication to the repository:

By removing the openapi from the committed files, we no longer need to create a pull request with those changes whenever the openapi chanegs on Horreum, instead we just need to trigger the ci (that will download the proper openapi from Horreum) and check that the build/test work

Awesome!!

We just need to configure Horreum to send some notification to this repo everytime we change something on our master or stable branches

Unfortunately with python there is no real guarantee that it would work because there is no compilation, therefore I think we should implement some tests that check ~everything is working

Yes, that would be good to have I think.

Other improvements are summarized in the pr description

webbnh

Hi @lampajr, here are some nits for you to consider.

.github/workflows/backport.yaml

webbnh · 2024-03-25T17:46:00Z

.github/workflows/backport.yaml

+          (github.event.action == 'closed' && contains(github.event.pull_request.labels.*.name, 'backport-squash'))
+          || (github.event.action == 'labeled' && contains(github.event.label.name, 'backport-squash'))


Again, I would factor out the common condition:

(github.event.action == 'closed' || github.event.action == 'labeled') && && contains(github.event.label.name, 'backport-squash')

although I doubt that you need the github.event.action check, because I don't think this will run if neither of those is true, per the check at line 21.

I need the github.event.action check as I am performing two different checks based on that result:

github.event.action == 'closed' --> check github.event.pull_request.labels.*.name

github.event.action == 'labeled' --> check github.event.label.name

NOTE: github.event.label.name is present if and only if the event is labeled

Ah, I see.

But, if github.event.action == 'labeled' is true, then github.event.pull_request.labels.*.name should contain the updated value (right?), so you can check that instead of github.event.label.name, which should let you simplify this to a single condition.

That's true, but if I check github.event.pull_request.labels.*.name when adding a label I am actually checking if at least one label with that name exists, which is true even if I am adding a new label and backport already exists on that pul request. This is not what we want as it would trigger the workflow multiple times.

Good point...but, I think that applies to my previous suggestion (which I shall withdraw 🙂).

In this spot, we've already decided to run the workflow, so the only question is whether to squash or not, and for that I think you can check github.event.pull_request.labels.*.name in either case.

See other comment #4 (comment) I don't think that is enough to check backport word. Anyway we are talking about minors so not a big deal

.github/workflows/backport.yaml

.github/workflows/check-openapi-change.yaml

.github/workflows/ci.yaml

webbnh · 2024-03-25T20:02:02Z

test/horreum_client_it.py

+@pytest.fixture()
+async def anonymous_client_without_check() -> HorreumClient:
+    print("Setting up anonymous client")
+    return await new_horreum_client(base_url="http://localhost:8080")
+
+
+@pytest.fixture()
+async def anonymous_client() -> HorreumClient:
+    print("Setting up anonymous client")
+    client = await new_horreum_client(base_url="http://localhost:8080")


I believe that you can have fixtures depend on other fixtures, in which case you can remove this duplication.

yeah they should work with nested fixtures, I think the whole test infrastructure should be optimized/refactored but I would do in ad-hoc PRs

For further improvements #5

test/horreum_client_it.py

lampajr · 2024-03-26T10:27:55Z

Hi @lampajr, here are some nits for you to consider.

Thanks @webbnh for all your suggestions!

With commit 1baa755 I should have addressed most of them (at least those I think required changes)

webbnh

Some followup comments and a few new small things on your update.

.github/workflows/backport.yaml

.gitignore

Makefile

README.md

webbnh · 2024-03-26T15:50:43Z

docs/RELEASE.md

+Push changes and tag:
+```bash
+git push origin main
+git push origin v0.12
+```


right now I would keep it as simple as possible (at least until we have a first package release) then we can improve it.

That's fair.

In the future all pushes to main could be converted to automated pull request creation

Sure, but, as a general rule, pushes to main and other "special branches" should be disallowed and forced to go through a pull request.

What is weird here is that this procedure implies that creating a release requires changes to the main branch. It really shouldn't -- you should be able to create a release from any point on main. Now, it is true that, once you create a release, any files on main which contain information related to the release name should probably be updated, but even that is not necessarily true (it depends on how you are naming your releases, and whether main carries a versioned release name, and whether the current contents of main are actually targeted at a new release).

webbnh · 2024-03-26T16:24:17Z

docs/RELEASE.md

+### Tag a new version
+
+Checkout to your branch, either a "stable" (e.g., `0.12.x`) or the `main` one.
+
+```bash
+git checkout origin/main
+```


You really should, first, do a command like

git remote update [--prune]

to ensure that the origin/main tracking branch is actually up to date with the remote origin repo before you create a local branch from it.

Then, when you do the checkout, you need to specify the name of the local branch to create (otherwise, at least in my environment, you end up in "detached HEAD state"):

git checkout origin/main -b main

(If you maintain a local main branch which you keep up to date with the origin/main by doing periodic pull's, perhaps this doesn't apply -- I don't keep a main branch, locally -- but otherwise I think you'll run into problems when you use a branch other than main if you don't already have it and maintain it locally.)

webbnh · 2024-03-26T16:31:16Z

docs/RELEASE.md

+To create a _stable_ branch from the `main` one, e.g., `0.12.x`, run the following commands.
+
+```bash
+git checkout origin/main -b 0.12.x
+git checkout origin/main
+```


Again, before checking out origin/main you need to make sure that it is up to date with the origin repo, either by doing a git remote update or a git pull.

And, when I check out the main branch, I need to specify the local branch name:

git checkout origin/main -b main

otherwise, I end up with a "detached HEAD", and the attempt to push my local main branch won't push the changes that poetry made (because they will be on my detached branch and not on main, if it even exists).

webbnh · 2024-03-26T16:39:02Z

docs/RELEASE.md

+git checkout origin/main
+git checkout -b 0.12.x


Assuming that you have a main branch and a 0.12.0 branch (or, if there are no unique commits on it, the 0.12.0 "branch" could just be a tagged commit on the main branch), if you then want to release 0.12.1 you have a choice: you can either add the pertinent commits on the 0.12.0 branch (creating a new branch if it was just a tag), or you can create a tag (0.12.1) for the commit on main which you want to release.

Life gets more interesting when you want to release 0.12.2, but it boils down to the the same choices: you can add new commits to the 0.12.1 branch (creating it if necessary, if it is just a tag) or you can tag the existing commit that you want to release on whichever branch it is on (0.12.1 or main).

There is nothing which requires you to create a branch in order to do a release, and there is nothing which demands that you release an update from an existing release branch. These choices are driven by the contents of the files and changes which comprise the release (and what the ancestry of those files' commits are).

But, the real point here is, instead of checking out/creating a branch, making no changes, and pushing it up, you can do that with a single push command, which is probably much simpler. E.g.,

git push origin origin/main:refs/heads/0.12.x

which creates the 0.12.x branch on origin using the contents of origin's main (assuming that your local tracking branch for origin/main is up to date with the remote origin).

test/horreum_client_it.py

lampajr · 2024-03-26T17:45:19Z

Thanks @webbnh for all suggestions 🙏

For sure the release process needs to be improved and automated but I think that it is worth to expand on that front while doing it, i.e., as soon as we try releasing a new python package I will keep note of all the steps and then we can easily automate it or at least include a well-detailed process.

Setup release process and README improvement

3adb58e

lampajr force-pushed the prepare_package branch from c9b8dd8 to 3adb58e Compare March 21, 2024 16:18

Simplify codebase and workflows

46ff8ce

lampajr force-pushed the prepare_package branch from 4a3b3ee to 46ff8ce Compare March 22, 2024 17:02

lampajr changed the title ~~Setup release process and README improvement~~ Setup release process and other improvements Mar 22, 2024

lampajr mentioned this pull request Mar 23, 2024

Which files should be included in this repo? #3

Closed

Setup test automation with nox

4045e90

lampajr force-pushed the prepare_package branch from 97988fa to 4045e90 Compare March 25, 2024 09:54

lampajr added 2 commits March 25, 2024 11:03

Minor fixes

6c59493

Added backporting to 0.12.x branch

1366f4e

lampajr marked this pull request as ready for review March 25, 2024 10:07

lampajr requested a review from stalep March 25, 2024 10:11

lampajr commented Mar 25, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Added maintainers

cb1b930

webbnh reviewed Mar 25, 2024

View reviewed changes

Applied suggestions

1baa755

webbnh reviewed Mar 26, 2024

View reviewed changes

Applied additional suggestions

6b774ac

stalep merged commit 5c76e0e into Hyperfoil:main Mar 27, 2024
3 checks passed

lampajr deleted the prepare_package branch March 27, 2024 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup release process and other improvements #4

Setup release process and other improvements #4

lampajr commented Mar 21, 2024 •

edited

Loading

lampajr commented Mar 25, 2024

stalep commented Mar 25, 2024

webbnh left a comment

webbnh Mar 25, 2024

lampajr Mar 26, 2024

webbnh Mar 26, 2024

lampajr Mar 26, 2024

webbnh Mar 26, 2024

lampajr Mar 26, 2024

webbnh Mar 25, 2024

lampajr Mar 26, 2024

lampajr Mar 26, 2024

lampajr commented Mar 26, 2024

webbnh left a comment

webbnh Mar 26, 2024

webbnh Mar 26, 2024

webbnh Mar 26, 2024

webbnh Mar 26, 2024

lampajr commented Mar 26, 2024

		(github.event.action == 'closed' && contains(github.event.pull_request.labels.*.name, 'backport-squash'))
		\|\| (github.event.action == 'labeled' && contains(github.event.label.name, 'backport-squash'))

Setup release process and other improvements #4

Setup release process and other improvements #4

Conversation

lampajr commented Mar 21, 2024 • edited Loading

Changes proposed

Check List (Check all the applicable boxes)

lampajr commented Mar 25, 2024

stalep commented Mar 25, 2024

webbnh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lampajr commented Mar 26, 2024

webbnh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lampajr commented Mar 26, 2024

lampajr commented Mar 21, 2024 •

edited

Loading