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

New Release Workflow #1590

Merged
merged 7 commits into from
May 12, 2022

Conversation

pSchlarb
Copy link
Member

hyperledger/indy-shared-gha#4 needs to be merged first for this PR to work.

release-workflow

Signed-off-by: pSchlarb <p.schlarb@esatus.com>
Signed-off-by: pSchlarb <p.schlarb@esatus.com>
building/publishing on .py changes

Signed-off-by: Philipp Schlarb <p.schlarb@esatus.com>
Signed-off-by: Philipp Schlarb <p.schlarb@esatus.com>
Signed-off-by: Philipp Schlarb <p.schlarb@esatus.com>
@pSchlarb pSchlarb requested a review from a team as a code owner May 10, 2022 12:41
@sovbot
Copy link
Contributor

sovbot commented May 10, 2022

Can one of the admins verify this patch?

Signed-off-by: Philipp Schlarb <p.schlarb@esatus.com>
@WadeBarnes WadeBarnes self-assigned this May 10, 2022
@pSchlarb pSchlarb added the Ubuntu 20.04 Ubuntu 20.04 related activity. label May 10, 2022
Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

Looks good, just a few small suggestions. I'm also going to test on my fork.

.github/workflows/README.md Outdated Show resolved Hide resolved
.github/workflows/PR.yaml Outdated Show resolved Hide resolved
.github/workflows/publishRelease.yaml Outdated Show resolved Hide resolved
.github/workflows/publishRelease.yaml Outdated Show resolved Hide resolved
.github/workflows/publishRelease.yaml Outdated Show resolved Hide resolved
.github/workflows/publishRelease.yaml Outdated Show resolved Hide resolved
@WadeBarnes
Copy link
Member

@pSchlarb, The following workflows have a very similar set of jobs, PR.yaml, Push.yaml, and releasepr.yaml. I think releasepr.yaml is different enough to stand on its own. PR.yaml and Push.yaml are almost identical though. I can also see why we want to keep them separate. I'm wondering whether is makes sense to turn the workflow-setup, lint, build-docker-image, plenum_tests, and build_packages into a reusable workflow that could be used by both. On its own in PR.yaml, and in combination with publish_artifacts in Push.yaml.

Do you think it's worth the extra effort at this point or should we mark it as a ToDo for the first time that "common" flow needs to be updated? I'm leaning toward the YAGNI approach and marking it as a ToDo.

Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

Ran into some dependency issues with ./bump_version.sh. Needed to install a few more minimal dependencies for it to work.

.github/workflows/tag.yaml Outdated Show resolved Hide resolved
Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

We don't want the workflow attempting to publish packages when it's running in someone's fork, since it will always fail.

.github/workflows/Push.yaml Show resolved Hide resolved
Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

We need to protect against publishing in forks here too.

.github/workflows/publishRelease.yaml Outdated Show resolved Hide resolved
@pSchlarb
Copy link
Member Author

@pSchlarb, The following workflows have a very similar set of jobs, PR.yaml, Push.yaml, and releasepr.yaml. I think releasepr.yaml is different enough to stand on its own. PR.yaml and Push.yaml are almost identical though. I can also see why we want to keep them separate. I'm wondering whether is makes sense to turn the workflow-setup, lint, build-docker-image, plenum_tests, and build_packages into a reusable workflow that could be used by both. On its own in PR.yaml, and in combination with publish_artifacts in Push.yaml.

The Problem with that idea is that Github doesn't allow subsequent workflow calls.
I would leave at is, since it is totally clear and the duplication doesn't hurt in the case here imho.

Copy link
Member

@WadeBarnes WadeBarnes left a comment

Choose a reason for hiding this comment

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

Looks like a line in one of the workflows was inadvertently duplicated.

Comment on lines 113 to 114
if: needs.release-infos.outputs.isVersionBump == 'true'
if: needs.release-infos.outputs.isVersionBump == 'true'
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate line. I think that was meant to be something like if: needs.release-infos.outputs.isVersionBump == 'true' && needs.release-infos.outputs.publish == 'true'

Signed-off-by: Philipp Schlarb <p.schlarb@esatus.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ubuntu 20.04 Ubuntu 20.04 related activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants