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

CI Pipeline: Refactor CI pipeline files #17933

Merged
merged 37 commits into from
Dec 8, 2020
Merged

Conversation

brbrr
Copy link
Contributor

@brbrr brbrr commented Dec 1, 2020

This is a follow up to GH Actions migration #17902

Changes proposed in this Pull Request:

  • move CI related files into .github folder
  • Refactor these files into a smaller, single purpose scripts

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

n/a

Testing instructions:

  • Does it still works?
  • Code looks good?

Proposed changelog entry for your changes:

  • n/a

@jetpackbot
Copy link

jetpackbot commented Dec 1, 2020

Scheduled Jetpack release: December 10, 2020.
Scheduled code freeze: December 3, 2020

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 4b017d6

@brbrr brbrr added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Dec 3, 2020
Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

While the test jobs are running, they're not actually running the tests. See inline comments for details.

I've suggested some pretty significant changes inline. Feel free to disagree, but IMO it'd be better to do more of the work in the workflow yaml files instead of in scripts to take advantage of the Github UI's grouping of steps. That'd get rid of the code-coverage.sh, phpunit.sh, and includes.sh files.

.github/actions/php-pipeline/code-coverage.sh Outdated Show resolved Hide resolved
.github/actions/php-pipeline/code-coverage.sh Outdated Show resolved Hide resolved
.github/actions/php-pipeline/code-coverage.sh Outdated Show resolved Hide resolved
.github/actions/php-pipeline/phpunit.sh Outdated Show resolved Hide resolved
.github/workflows/code-coverage.yml Outdated Show resolved Hide resolved
.github/workflows/phpunit.yml Outdated Show resolved Hide resolved
.github/actions/php-pipeline/setup-env.sh Outdated Show resolved Hide resolved
.github/actions/php-pipeline/setup-env.sh Outdated Show resolved Hide resolved
.github/actions/php-pipeline/phpunit.sh Outdated Show resolved Hide resolved
.github/actions/php-pipeline/setup-env.sh Show resolved Hide resolved
@anomiex anomiex added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 3, 2020
@brbrr brbrr force-pushed the update/refactor-ci-pipeline-files branch from 81eba62 to 3527f5e Compare December 4, 2020 16:33
@brbrr brbrr added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Dec 7, 2020
@brbrr brbrr requested a review from anomiex December 7, 2020 11:33
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Dec 7, 2020
@brbrr
Copy link
Contributor Author

brbrr commented Dec 7, 2020

@anomiex Thanks for the thorough review! (I think) I addressed all your comments and move away from shell scripts completely.

@anomiex anomiex added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 7, 2020
anomiex added a commit that referenced this pull request Dec 7, 2020
We're passing "8.0" rather than "nightly", so look for that.

PR #17933 is going to change all that anyway, but it's not quite ready
yet.
@brbrr brbrr force-pushed the update/refactor-ci-pipeline-files branch from a6a8340 to 587372a Compare December 8, 2020 10:12
.github/workflows/phpunit.yml Outdated Show resolved Hide resolved
.github/workflows/code-coverage.yml Outdated Show resolved Hide resolved
.github/workflows/code-coverage.yml Show resolved Hide resolved
@brbrr brbrr added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Dec 8, 2020
@brbrr brbrr force-pushed the update/refactor-ci-pipeline-files branch from f2ef8aa to 4b017d6 Compare December 8, 2020 16:03
@anomiex anomiex added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 8, 2020
@brbrr brbrr merged commit b33b2f7 into master Dec 8, 2020
@brbrr brbrr deleted the update/refactor-ci-pipeline-files branch December 8, 2020 16:15
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 8, 2020
@github-actions github-actions bot added this to the 9.2.1 milestone Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build [Status] Needs Package Release This PR made changes to a package. Let's update that package now. [Type] Infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants