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 custom GitHub actions for reuse across repositories #11

Merged
merged 4 commits into from
Jun 15, 2022

Conversation

eason9487
Copy link
Member

@eason9487 eason9487 commented Jun 7, 2022

Changes proposed in this Pull Request:

It's phase 1 of the migration plan in this comment - pb0Spc-2DL-p2#comment-5753.

Add three custom GitHub actions for reuse across repositories.

Detailed test instructions:

Go to the Files changed and Check tabs of the RP 1126-gh-woocommerce/automatewoo to see how these custom actions are used.

Additional details:

About the custom composite action: https://docs.github.com/en/actions/creating-actions/creating-a-composite-action

Changelog entry

Add - Custom GitHub actions for reuse across repositories.

@eason9487 eason9487 requested a review from a team June 7, 2022 05:55
@eason9487 eason9487 self-assigned this Jun 7, 2022
@puntope
Copy link
Contributor

puntope commented Jun 7, 2022

Howdy @eason9487

One question before testing. I guess this PR just stores the files here and the developer should copy paste them to the projects?

I'm thinking if would be more convenient to extend the generator adding some kind of GH actions generator

In my mind would be some prompt like (just an example to get the idea):

  • Do you want to install GH Actions? yes/no
  • Is your project using node? Yes/No
  • Is your project using php? Yes/No
  • Is your project using mysql? Yes/No

Then depending on answers the files would be generated in the folder.

@tomalec
Copy link
Member

tomalec commented Jun 7, 2022

AFAIU, individual repos do not copy those files but use a reference to here
1126-gh-woocommerce/automatewoo/files#diff-626cf1025fab6d0ecd9b6b485d250f06348b4893aae8b6ad792ac6b409000490R55

      - name: Prepare PHP
        uses: woocommerce/grow/github-actions/prepare-php@github-actions
        with:
          php-version: "${{ matrix.php }}"
          install-deps: "no"

I like the idea of having a generator for workflows as well, but I'd keep actions here if they are totally same for all the repos.

💅 Then we could create another PR to add workflow generator, that would setup more composite, repo-specific workflows, like 1126-gh-woocommerce/automatewoo/files#diff-626cf1025fab6d0ecd9b6b485d250f06348b4893aae8b6ad792ac6b409000490

  • PHP Coding Standards
  • PHP Unit tests
  • (TBC) PR autolabeler

uses: actions/checkout@v3

- name: Prepare MySQL
uses: woocommerce/grow/github-actions/prepare-mysql@github-actions
Copy link
Member

@tomalec tomalec Jun 7, 2022

Choose a reason for hiding this comment

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

Maybe it would be safer to use commit hash, to avoid problems like woocommerce/google-listings-and-ads#1552, especially that this branch may get removed after this PR is merged.

However, branch names are much easier to read 😕

Maybe change to trunk?

Seems we need to solve #5 sooner than later

Copy link
Member Author

@eason9487 eason9487 Jun 8, 2022

Choose a reason for hiding this comment

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

Phases 4 and 5 in pb0Spc-2DL-p2#comment-5753 are creating the versioning management for the GitHub actions in this repo. With the versioning feature, the caller repos will use @v1-actions to apply actions instead of a specific commit or a branch.

The @v1-actions will keep pointing to the latest released v1.x.x-actions tag, Thus, caller repos will always have the latest actions by using the @v1-actions major version.

I have completed the development of versioning management in the add/versioning-action branch and tested it with a demo release. In the PR 139-gh-woocommerce/automatewoo-referrals, we can find that the uses of uses: woocommerce/grow/github-actions/prepare-php@v1-actions-pre work well.

All the uses in all caller repos will be changed to uses: woocommerce/grow/github-actions/<action-directory>@v1-actions once the feature is formally released. I will create a separate PR for the versioning management later.

@eason9487
Copy link
Member Author

eason9487 commented Jun 8, 2022

@puntope

One question before testing. I guess this PR just stores the files here and the developer should copy paste them to the projects?

Sorry, I missed mentioning the context of this PR. It's phase 1 of the migration plan in this comment - pb0Spc-2DL-p2#comment-5753.

These actions would not be coped to caller repos but only be used like these examples. The way it is actually used can be found in this AW PR - 1126-gh-woocommerce/automatewoo.

@eason9487
Copy link
Member Author

@tomalec and cc @puntope

I like the idea of having a generator for workflows as well, but I'd keep actions here if they are totally same for all the repos.

For the current scope of migrating Travis CI to GitHub Actions, I think only the Coding Standards would be completely the same or at least highly repetitive. The PHP Unit tests workflows like AW, AW-Referrals, and GLA are roughly similar but much more details are different depending on each repo.

💅 Then we could create another PR to add workflow generator, that would setup more composite, repo-specific workflows, [...]

I planned to complete the Travis CI to GitHub Actions migration first, and then we could have a more clear picture of what workflows have the better fit to have generators.

@eason9487 eason9487 requested a review from tomalec June 8, 2022 03:11
Copy link
Member

@tomalec tomalec left a comment

Choose a reason for hiding this comment

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

Reviewed the code, and action logs, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants