nienbo / cache-buildkite-plugin

Tarball, Rsync & S3 Cache Kit for Buildkite. Supports Linux, macOS and Windows
https://buildkite.com/plugins
MIT License
67 stars 39 forks source link

Fix advanced example where `plugins:` has to be array #28

Closed ciuncan closed 3 years ago

ciuncan commented 3 years ago

This PR fixes the advanced example in README.md where previously using <<: *caches yielded an object which collapses all plugins into a single field ("gencer/cache#v2.4.8")also buildkite rejecting that definition; after the change it creates an array of plugin definitions as required by buildkite.

gencer commented 3 years ago

Merged. Teşekkürler!

gencer commented 3 years ago

@ciuncan Buildkite Linter complains about this change: https://github.com/gencer/cache-buildkite-plugin/runs/2468713133?check_suite_focus=true

You may want to see that and perhaps needs a second PR.

gencer commented 3 years ago

You can check your markup here:

ciuncan commented 3 years ago

Hi @gencer , thanks for the quick merge! And sorry, I didn't check the example properly after sending a quick force-pushed commit second time. My example only consisted of the the - *caches array, I guess YAML doesn't support additional items when a reference to an array is being used.

If it is OK for you, I can open a new PR where I move the docker plugin into caches array and rename it as all-plugins as below. Would that work for you?

node-cache: &node-cache
  id: node
  key: "v1-cache-{{ id }}-{{ runner.os }}-{{ checksum 'yarn.lock' }}"
  restore-keys:
    - 'v1-cache-{{ id }}-{{ runner.os }}-'
    - 'v1-cache-{{ id }}-'
  backend: s3
  s3:
    bucket: cache-bucket
  paths:
    - node_modules
    # If you have sub-dir then use:
    # - **/node_modules

ruby-cache: &ruby-cache
  id: ruby
  key: "v1-cache-{{ id }}-{{ runner.os }}-{{ checksum 'Gemfile.lock' }}"
  restore-keys:
    - 'v1-cache-{{ id }}-{{ runner.os }}-'
    - 'v1-cache-{{ id }}-'
  backend: s3
  s3:
    bucket: cache-bucket
  paths:
    - 'bundler/vendor'

all-plugins: &all-plugins
  - gencer/cache#v2.4.8: *node-cache
  - gencer/cache#v2.4.8: *ruby-cache
  - docker#v3.7.0: ~ # Use your config here

steps:
  - name: ':jest: Run tests'
    key: test
    command: yarn test --runInBand && bundle exec rspec --color
    plugins: *all-plugins
  - name: ':istanbul: Run Istanbul'
    key: istanbul
    depends_on: test
    command: .buildkite/steps/istanbul.sh
    plugins: *all-plugins
gencer commented 3 years ago

Hi @ciuncan That was my reason why I used object at first place. Even Buildkite says invalid, It just works™.

However, To be more accurate and be able to pass linting on Actions, Let's do this.

Feel free to send a new PR. 🚀

P.S.: You probably need to bump your master (rebase) as I've enabled linting on Actions CI.

ciuncan commented 3 years ago

For me it was failing with the below error message at pipeline upload step with when plugins was an object/dictionary:

2021-04-29 11:39:41 FATAL  Pipeline parsing of "build-and-test.pipeline.yml" failed (Failed to parse build-and-test.pipeline.yml: map merge requires map or sequence of maps as the value)

Maybe they changed into array strictly later? I made sure it conforms to following examples. https://buildkite.com/docs/plugins/using#adding-a-plugin-to-your-pipeline

Anyways, I opened the following PR: https://github.com/gencer/cache-buildkite-plugin/pull/30

gencer commented 3 years ago

Ah. I probably missed that! Thanks for pointing it out for me.

I'm glad we fixed this! Thanks again! 🎉