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

Including optional shard installation #7

Open
stephendolan opened this issue Aug 3, 2021 · 1 comment
Open

Including optional shard installation #7

stephendolan opened this issue Aug 3, 2021 · 1 comment

Comments

@stephendolan
Copy link

stephendolan commented Aug 3, 2021

First of all, thank you for providing this amazing GitHub Action! It's been a really great addition to my CI workflows.

Something I've found a lot of value in with the ruby/setup-ruby action is the ability to run bundle install and cache the gems as a part of the action with the bundler-cache: true option.

Is something like a shards-cache: true option something that the maintainers would be open to as a part of this action? I haven't worked with writing GitHub actions before, so I wanted to make sure there was an appetite to review a PR before taking the time to learn and implement one.

This would reduce the common copy-and-paste that I do to every Crystal project, which generally does these steps every time:

steps:
  - uses: actions/[email protected]

  - uses: crystal-lang/install-crystal@v1
    with:
      crystal: latest

  - name: Set up Crystal cache
    uses: actions/[email protected]
    id: crystal-cache
    with:
      path: |
        ~/.cache/crystal
        bin/ameba
        lib
      key: ${{ runner.os }}-crystal-${{ matrix.crystal_version }}-${{ hashFiles('**/shard.lock') }}
      restore-keys: |
        ${{ runner.os }}-crystal-${{ matrix.crystal_version }}

  - name: Install shards
    if: steps.crystal-cache.outputs.cache-hit != 'true'
    run: shards check || shards install --ignore-crystal-version

  - name: Run tests
    run: crystal spec

My hope would be that these steps could be reduced to (naming TBD):

steps:
  - uses: actions/[email protected]

  - uses: crystal-lang/install-crystal@v1
    with:
      crystal: latest
      shards-cache: true
      shards-cache-ignore-crystal-version: true
      shards-cache-additional-paths: bin/ameba

  - name: Run tests
    run: crystal spec

Or, if you didn't use something like Ameba and didn't do matrix testing against old Crystal versions, something as simple as:

steps:
  - uses: actions/[email protected]

  - uses: crystal-lang/install-crystal@v1
    with:
      crystal: latest
      shards-cache: true

  - name: Run tests
    run: crystal spec
@oprypin
Copy link
Member

oprypin commented Aug 8, 2021

Hi.
This action was first developed when caching by custom actions was not only an extreme rarity, but there wasn't even API access to it. Now the situation is totally opposite and indeed I am convinced that caching shards should be a part of this.

I may be opposed to adding even more extra features. E.g. shards-cache-additional-paths I would not like because it merely clumps 2 separate steps into one without any benefit.

And, some thought needs to be given regarding what to cache. E.g. https://oprypin.github.io/install-crystal/configurator.html suggests 2 styles: cache ~/.cache/shards or ./lib depending on the circumstances.

Finally, a pull request. It would be good, thanks. Although, I am likely to be picky regarding the approach. In other case, eventually I will get around to this myself.

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

No branches or pull requests

2 participants