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

Add publish-post-release.yml CI workflow #3794

Closed
wants to merge 2 commits into from
Closed

Conversation

vvv
Copy link
Contributor

@vvv vvv commented May 30, 2023

New CI workflow is triggered when a release is published, or a pre-release
is changed to a released. It can also be triggered manually.
publish-post-release.yml calls publish-binaries.yml and
publish-docker-images.yml --- they are made reusable for this purpose.
Once both workflows succeed, publish-apt-brew-pkg.yml is called.

Closes #3146

Other changes

  • Remove stray matrix reference in .github/workflows/sdks-tests.yml.

@@ -122,7 +122,7 @@ jobs:
- name: Install pipenv
uses: dschep/install-pipenv-action@v1
- name: Install dependencies
run: pipenv install --dev --python=${{ matrix.python-version }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

matrix is not defined.

on:
release:
types: [released]
workflow_dispatch:
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 guess the ability to start this CI workflow manually might come handy?

# This workflow is reusable, i.e. it may be called from another workflow.
# See https://docs.github.com/en/actions/using-workflows/reusing-workflows
workflow_call:
workflow_dispatch:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is optional. It might be useful to be able to run this workflow manually though.

.github/scripts/check-release.sh Outdated Show resolved Hide resolved
.github/scripts/check-release.sh Outdated Show resolved Hide resolved
.github/scripts/check-release.sh Outdated Show resolved Hide resolved
.github/scripts/check-release.sh Outdated Show resolved Hide resolved
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Hello @vvv

just a quick review before a deeper one: can you put the changes related to the check-release.sh script in another PR?

Thanks again for the PR
It's not related to the purpose of this one, if I understand correctly.

@vvv
Copy link
Contributor Author

vvv commented May 31, 2023

can you put the changes related to the check-release.sh script in another PR?
[...] It's not related to the purpose of this one, if I understand correctly.

@curquiza Sure. Do I need to create an issue for that, or a PR will suffice?

@vvv
Copy link
Contributor Author

vvv commented May 31, 2023

@curquiza I've moved check-release.sh edits to separate PR — #3799. Thanks for consideration.

@vvv
Copy link
Contributor Author

vvv commented Jun 12, 2023

@curquiza Gentle reminder. 😇

@curquiza
Copy link
Member

Hello @vvv
Thanks for being around 😁
I really appreciate your work but the impact of this PR is huge and not my priority so far (the current CIs are working ok so not at top of my emergencies): I do my best to review and make a decision about it by the end of June but cannot guarantee anything.
Thanks again for your involvement. Our team is small and we try to prioritize and focus on the most impactful work without giving up the community at the same time, but not always easy 😊

@vvv
Copy link
Contributor Author

vvv commented Jun 12, 2023

@curquiza Understood. Thanks for the explanation.

However small Meilisearch team is, you folks are doing sterling work at prioritizing and focusing! The product looks great, the documentation is a beaut, and my contribution experience has been nothing but pleasant so far. ❤️

vvv added 2 commits July 2, 2023 18:01
New CI workflow is triggered when a release is published, or a pre-release
is changed to a released. It can also be triggered manually.
`publish-post-release.yml` calls `publish-binaries.yml` and
`publish-docker-images.yml` --- they are made reusable for this purpose.
Once both workflows succeed, `publish-apt-brew-pkg.yml` is called.

Closes meilisearch#3146
@curquiza
Copy link
Member

curquiza commented Jul 3, 2023

Hello @vvv
Thanks again for your PR
I'm going to be transparent: I don't feel confident merging this. Not because of the quality of your work, or anything, but because it will change something that already works, by making it more complex (even if it's just a little bit) to fix a small issue we have.
I'm afraid if it does not work immediately (it never does in the CI world 😂) I have to spend time trying to understand what is happening and fixing this. A time I currently don't have. I realize it's really convenient I currently know how our CIs work, including in edge-case situations. It would not be strategic to lose this, especially because we will probably change our workflow in the future, and I have to master our current CI process to adapt it to our new decisions.

In any case, I keep your solution in mind: I get your suggestions, I just don't know how it works in every situation, and I'm sure I will be able to refer to it in the future.

Sorry to close this PR, and thank you again for your time. It's definitely not lost ❤️

@curquiza curquiza closed this Jul 3, 2023
@vvv
Copy link
Contributor Author

vvv commented Jul 3, 2023

@curquiza No problem. I trust your judgment. Cheers!

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.

Homebrew CI: run only after docker image and binaries are published
2 participants