crystal-lang / install-crystal

GitHub Action: Install Crystal programming language
https://crystal-lang.github.io/install-crystal/configurator.html
MIT License
67 stars 8 forks source link

Including optional shard installation #7

Open stephendolan opened 3 years ago

stephendolan commented 3 years ago

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/checkout@v2.3.4

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

  - name: Set up Crystal cache
    uses: actions/cache@v2.1.6
    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/checkout@v2.3.4

  - 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/checkout@v2.3.4

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

  - name: Run tests
    run: crystal spec
oprypin commented 3 years ago

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.