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.

Sign up for GitHub

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

Jump to bottom

[#13] reuse open-api-workflows workflows #189

Merged
merged 5 commits into from
Oct 17, 2024
Merged

[#13] reuse open-api-workflows workflows #189

merged 5 commits into from
Oct 17, 2024

Conversation

SonnyBA
Copy link
Contributor

@SonnyBA SonnyBA commented Sep 17, 2024

Fixes maykinmedia/open-api-framework#13

Changes

Reuses open-api-workflows workflows to make CI jobs consistent across the different projects.

@SonnyBA SonnyBA self-assigned this Sep 17, 2024
@SonnyBA SonnyBA marked this pull request as ready for review September 17, 2024 13:48
@SonnyBA SonnyBA mentioned this pull request Sep 17, 2024
Closed
2 tasks
Coperh
Coperh approved these changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@Coperh Coperh left a comment

Choose a reason for hiding this comment

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

Also looks fine.

Same comment about the open-api-workflows not being the main branch

@SonnyBA SonnyBA force-pushed the feature/13-consistent-ci-configuration branch from fc7dea2 to 1e0ee96 Compare October 4, 2024 10:09
stevenbal
stevenbal approved these changes Oct 7, 2024
View reviewed changes
Copy link
Contributor

@stevenbal stevenbal 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 minor question regarding the workflow name

.github/workflows/ci.yml

publish:
open-api-ci:
uses: maykinmedia/open-api-workflows/.github/workflows/ci.yml@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the full path needed here? The maykinmedia/setup-django-backend@v1.3 doesn't seem to have 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.

Yeah, the maykinmedia/setup-django-backend@v1.3 is an composite action. Where the changes in the PR use reusable workflows.

@SonnyBA
Copy link
Contributor Author

SonnyBA commented Oct 14, 2024

@joeribekker This repository still has the requirement of a successful previous "Docker image build" job run in order to merge any pull request. Since that job now moved to the open-api-workflows repository this needs to be changed (in order to merge this PR). The documentation mentions (in step 4) how this could be configured.

@SonnyBA SonnyBA mentioned this pull request Oct 17, 2024
Merged
@annashamray
Copy link
Contributor

@SonnyBA I've added maykinmedia/* into action permissions in the Settings, but it hasn't helped. The problem was in required statuses for the protected branch (main), Now it should work

@SonnyBA SonnyBA merged commit a5deb0b into main Oct 17, 2024
18 checks passed
@SonnyBA SonnyBA deleted the feature/13-consistent-ci-configuration branch October 17, 2024 14:22
@SonnyBA
Copy link
Contributor Author

SonnyBA commented Oct 17, 2024

Thanks @annashamray 👍, I can't get into the Settings tab, probably a permissions thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Coperh Coperh Coperh approved these changes

@stevenbal stevenbal stevenbal approved these changes

@annashamray annashamray annashamray approved these changes

@bart-maykin bart-maykin Awaiting requested review from bart-maykin

Assignees

@SonnyBA SonnyBA

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make CI pipelines the same across projects
4 participants
@SonnyBA @annashamray @Coperh @stevenbal