README: clarify that `cache-hit` returns a string and not a boolean by andreasabel · Pull Request #1263 · actions/cache · GitHub
Skip to content

README: clarify that cache-hit returns a string and not a boolean#1263

Closed
andreasabel wants to merge 2 commits into
actions:mainfrom
andreasabel:cache-hit-README
Closed

README: clarify that cache-hit returns a string and not a boolean#1263
andreasabel wants to merge 2 commits into
actions:mainfrom
andreasabel:cache-hit-README

Conversation

@andreasabel

Copy link
Copy Markdown

Rationale: if cache-hit was a boolean, then ! could be used to get the opposite.
However, it is really a string, and this should be pointed out clearly in the documentation.
Because both !'true' and !'false' are false, so customers can shoot themselves in the foot here.

Closes #1262.

@andreasabel

Copy link
Copy Markdown
Author

Comment thread README.md Outdated
* `cache-hit` - A string `'true'` or `'false'` indicating whether an exact match was found for the key.

> **Note** `cache-hit` will only be set to `true` when a cache hit occurs for the exact `key` match. For a partial key match via `restore-keys` or a cache miss, it will be set to `false`.
> **Note** `cache-hit` will only be set to `'true'` when a cache hit occurs for the exact `key` match. For a partial key match via `restore-keys` or a cache miss, it will be set to `'false'`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is still incorrect. On a cache miss, the output is not set, i.e. empty string.

@andreasabel andreasabel Mar 21, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@itchyny So you are saying it is three-valued, 'true', 'false' or empty?

@itchyny itchyny Mar 22, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the thorough investigation @itchyny ! I updated the text accordingly.

Rationale: if `cache-hit` was a boolean, then `!` could be used to get the opposite.
However, it is really a string, and this should be pointed out clearly in the documentation.
Because both `!'true'` and `!'false'` are `false`, so customers can shoot themselves in the foot here.

Closes actions#1262.
@joshmgross

Copy link
Copy Markdown
Contributor

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.

!cache-hit is false when cache-hit is false

4 participants