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

[Housekeeping] DRY CI with Action Composition #1518

Closed
8 tasks
MadsBuchmann opened this issue Apr 28, 2021 · 7 comments
Closed
8 tasks

[Housekeeping] DRY CI with Action Composition #1518

MadsBuchmann opened this issue Apr 28, 2021 · 7 comments
Labels
housekeeping NOT Prioritized Issue not yet prioritized and added to a Milestone stale Automatically applied when there is no activity on an issue or PR

Comments

@MadsBuchmann
Copy link
Contributor

MadsBuchmann commented Apr 28, 2021

Short description of housekeeping request

I would like to investigate if we can DRY our GitHub Actions workflows (located in .github/workflows) by utilising composite run steps and actions:

We have quite a lot of repeating steps. For example, if we are to change the node version used, we have to change it 3 - 5 different places.

This makes it easy to make mistakes and makes it hard to get an overview.

Tasks

Kick Off

  • Ensure this issue has been Tech refined with @kirbydesign/kirby-guild and is updated with a clear implementation description
    This issue should be in the Ready to do column of the Kirby kan-ban board before starting implementation
  • Assign yourself to this issue and move it to the In progress column of the Kirby kan-ban board

Code

  • Create Feature Branch from master branch
  • Implement/update unit tests
  • Create a draft implementation and push to Github
  • Ask a member of @kirbydesign/kirby-guild for a WIP review by creating a draft Pull Request

Code Review

  • Open a pull request (or mark the existing draft PR as Ready for review) and ask @kirbydesign/kirby-guild for a review
    Remember to add closes #issueno to the description of the PR.
  • Once approved, merge feature branch/PR to master

🎉 Celebrate

@MadsBuchmann MadsBuchmann added NOT Tech refined Needs Tech kickoff - solution outlined and agreed NOT Prioritized Issue not yet prioritized and added to a Milestone 👶🏻 New For new issues before prioritisation and refinement housekeeping labels Apr 28, 2021
@MadsBuchmann MadsBuchmann removed NOT Tech refined Needs Tech kickoff - solution outlined and agreed 👶🏻 New For new issues before prioritisation and refinement labels May 3, 2021
@MadsBuchmann MadsBuchmann self-assigned this May 3, 2021
@jakobe jakobe removed the NOT Prioritized Issue not yet prioritized and added to a Milestone label May 12, 2021
@MadsBuchmann
Copy link
Contributor Author

MadsBuchmann commented May 12, 2021

I am marking this as blocked - and leaving a little writeup for whoever will take on this issue in the future 😄

Why block it?

When starting this issue I tought it would be a problem storing composite actions in the same repo and not having to put them in an external one. This was quite easy actually as Github supports local composite actions:

    - name: Checkout repo    
      uses: actions/checkout@v2    
    - name: Say Hello                                                                                                                                                                                 
      uses: ./.github/actions/say-hello    
      with:        
        name: Mads   

It unfortunately turns out that you can't have uses steps in your composite actions. In other words, you are not able to use other Github Actions such as: actions/cache@v2 or actions/checkout@v2 etc.

It seems like It is a work in progress and something Github want to do but it is not ready yet. We should therefore leave this blocked until that feature is ready.

For more see the following resources:

An Alternative solution!

An alternative solution I've stumbled upon when working with this - is to implement YAML anchors via a build script that can be executed pre-commit. I found it here and the author also links to an example repo: actions/runner#438 (comment)

I played around with it for maybe 30 minutes but had some problems with my node version. I decided to not use anymore time on it as I do not know how this alternative solution will be received 😄

Ideas for Composite Actions

I've located two routines that are being executed several times in our workflows that I would like to turn into reusable composite actions. Unfortunately both of these uses other Github Actions and can therefore not be put into a composite run step.

The first I would like to put into a 'Setup Kirby environment' action:

      - name: Install Node.js    
        uses: actions/setup-node@v2    
        with:    
          node-version: ${{ env.NODE_VERSION }}    
      - name: Fetch Node Modules Cache    
        uses: actions/cache@v2    
        id: cache    
        with:    
          path: |     
            **/node_modules    
          key: node-modules-${{ runner.os }}-${{env.NODE_VERSION}}-${{hashFiles('**/package.json')}}-${{hashFiles('**/package-lock.json')}}    
      - name: Clean Install NPM Dependencies    
        if: steps.cache.outputs.cache-hit != 'true'    
        run: npm ci    
      - name: Run Postinstall Script    
        if: steps.cache.outputs.cache-hit == 'true'    
        run: npm run postinstall     

The above is executed around 5 times in our workflows at the time of writing.

Secondly there are 4 places in the code where very similar steps are executed, these are:

  • "Set deployment status to in progress"
  • "Set deployment status to failed"
  • "Set deployment status to success"
  • "Set deployment status to inactive".

Therefore I would like to create a "Set deployment status" action which should take a status as input.

@MadsBuchmann MadsBuchmann changed the title [Housekeeping] Composite Run Steps in CI [Blocked][Housekeeping] Composite Run Steps in CI May 12, 2021
@MadsBuchmann MadsBuchmann added the NOT Prioritized Issue not yet prioritized and added to a Milestone label May 12, 2021
@stale
Copy link

stale bot commented Jul 21, 2021

This issue has been automatically marked as stale because of no recent activity. It will be closed in 10 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Automatically applied when there is no activity on an issue or PR label Jul 21, 2021
@MadsBuchmann
Copy link
Contributor Author

In honor of the backlog pruning being done by @alxzak I'm closing this issue. We can get back to it whenever it is relevant.

More info: link

@MadsBuchmann MadsBuchmann changed the title [Blocked][Housekeeping] Composite Run Steps in CI [Housekeeping] DRY CI with Action Composition Sep 21, 2021
@MadsBuchmann
Copy link
Contributor Author

It is now possible to compose actions!
https://github.blog/changelog/2021-08-25-github-actions-reduce-duplication-with-action-composition/

@MadsBuchmann MadsBuchmann reopened this Sep 21, 2021
@stale stale bot removed the stale Automatically applied when there is no activity on an issue or PR label Sep 21, 2021
@MadsBuchmann MadsBuchmann removed their assignment Sep 21, 2021
@MadsBuchmann
Copy link
Contributor Author

MadsBuchmann commented Nov 1, 2021

Suggestions from the review of #1402 that could be carried out as part of this issue

1 )

Original comment from PR:

Now that GitHub supports action composition we could consider plopping line 20 to 46 into some sort of "setup" action. Then we could also replace it in the other files.

Might not be related to this issue... Buuuuut we do have the file open anyways 👀

Alternatively I can open a new issue and fix it there, which can then be merged into this branch.

Related code: .github/workflows/publish.yml

2 )

Original comment from PR:

Consider plopping line 54 to 76 into some sort of "bump and publish core" composite action. This will make it easier to get an overview of the code and afaik you will only have to write if: steps.modified-files.outputs.core == 'true' once.

Related code: .github/workflows/publish.yml

@stale
Copy link

stale bot commented Jan 10, 2022

This issue has been automatically marked as stale because of no recent activity. It will be closed in 10 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Automatically applied when there is no activity on an issue or PR label Jan 10, 2022
@stale
Copy link

stale bot commented Mar 21, 2022

This issue has been closed due to inactivity. Please open a new issue if it becomes relevant again.

@stale stale bot closed this as completed Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping NOT Prioritized Issue not yet prioritized and added to a Milestone stale Automatically applied when there is no activity on an issue or PR
Projects
None yet
Development

No branches or pull requests

2 participants