Skip to content

Conversation

@mgruner
Copy link
Contributor

@mgruner mgruner commented Feb 14, 2025

@mgruner
Copy link
Contributor Author

mgruner commented Feb 14, 2025

@DmitryTsepelev can you have a look, please? The code doesn't exactly look nice, but I tried to keep changes minimal and according to the existing style. If this is the right direction then I can also try to add something to the README.md.

Copy link
Owner

@DmitryTsepelev DmitryTsepelev left a comment

Choose a reason for hiding this comment

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

I like it a lot! Please proceed with docs and we'll merge it in

@mgruner mgruner marked this pull request as ready for review February 18, 2025 13:50
@mgruner
Copy link
Contributor Author

mgruner commented Feb 18, 2025

@DmitryTsepelev documentation added.

When testing this with our actual use case in our software https://github.com/zammad/zammad, I found a bug in my initial code and also issues in the tests which caused some more changes, also to the tests to ensure that the use case really gets tested properly.

To me it looks good now, and it also works correctly in our use case. But please have another detailed look again if I got everything right. I took the cache_key: { exclude_arguments: … } syntax from your proposal. Is this the right signature that should get used in the gem?

If everything looks right to you, feel free to merge it, and please let us know when you plan to release a new version.

@mgruner
Copy link
Contributor Author

mgruner commented Feb 19, 2025

@DmitryTsepelev sorry, rubocop corrected.

@DmitryTsepelev DmitryTsepelev merged commit 67424d0 into DmitryTsepelev:master Feb 20, 2025
18 checks passed
@DmitryTsepelev
Copy link
Owner

LGTM, I'll make a release right now

@mgruner
Copy link
Contributor Author

mgruner commented Feb 20, 2025

Thank you!

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.

Possibility to filter out arguments from path_cache_key

2 participants