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 support to cache on specific json keys #51

Merged
merged 10 commits into from
Apr 9, 2024
Merged

Conversation

samchungy
Copy link
Contributor

@samchungy samchungy commented Apr 2, 2024

@72636c 72636c mentioned this pull request Apr 2, 2024
@samchungy samchungy marked this pull request as ready for review April 4, 2024 01:57
@samchungy samchungy requested a review from 72636c April 4, 2024 02:35
Copy link

@AaronMoat AaronMoat left a comment

Choose a reason for hiding this comment

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

I can't spot anything wrong, but
image

Copy link
Member

@72636c 72636c left a comment

Choose a reason for hiding this comment

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

👏

- package.json#.devDependencies
- package.json#.pnpm.overrides
- yarn.lock
- docker#v3.12.0:
Copy link
Member

Choose a reason for hiding this comment

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

Might be time to renovate these references 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll tweak them in a separate PR, too many changes otherwise

README.md Outdated
@@ -76,6 +76,23 @@ steps:
- /workdir/node_modules
```

It also supports caching on specific JSON keys which can be specified following a `#` character using [jq syntax](https://jqlang.github.io/jq/manual/#object-identifier-index). This requires [jq](https://jqlang.github.io/jq/) to be installed on the build agent. You cannot use this in conjunction with Bash globbing.
Copy link
Member

Choose a reason for hiding this comment

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

More for our internal understanding, is it worth clarifying

  • The implementation matches on the first .json# substring, which would be silly to have elsewhere in your file path but is nonetheless possible on Unix systems
  • A given entry cannot mix globbing and jq paths, but you can do ['**/LICENSE.md', 'package.json#.dependencies']

README.md Outdated
Comment on lines 83 to 90
- command: npm test
plugins:
- seek-oss/docker-ecr-cache#v2.2.0:
cache-on:
- package.json#.dependencies
- package.json#.devDependencies
- package.json#.pnpm.overrides
- yarn.lock
Copy link
Member

Choose a reason for hiding this comment

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

Would it be useful to list our recommended set of cache keys for a typical pnpm project here?

Suggested change
- command: npm test
plugins:
- seek-oss/docker-ecr-cache#v2.2.0:
cache-on:
- package.json#.dependencies
- package.json#.devDependencies
- package.json#.pnpm.overrides
- yarn.lock
- command: pnpm install --offline && pnpm test
plugins:
- seek-oss/docker-ecr-cache#v2.2.0:
cache-on:
- .npmrc
- package.json#.dependencies
- package.json#.devDependencies
- package.json#.packageManager
- package.json#.pnpm.overrides
- pnpm-lock.yaml

@samchungy samchungy merged commit f8e70fb into master Apr 9, 2024
2 checks passed
@samchungy samchungy deleted the cache-specific-keys branch April 9, 2024 01:26
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

Successfully merging this pull request may close these issues.

3 participants