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

Allow all possible args in tagged_with method #2211

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

robinboening
Copy link
Contributor

@robinboening robinboening commented Nov 5, 2021

Previously, we only allowed tag names to be passed into the tagged_with method. This commit allows to take advantage of the interface gutentag has to offer, while being backwards compatible.

Closes #2208

If we agree on this proposed implementation I can add a couple of specs to cover the change.

@robinboening robinboening requested a review from a team November 5, 2021 17:06
@robinboening robinboening force-pushed the extend_tagged_with_interface branch from 2926970 to bc42f3a Compare November 5, 2021 17:07
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

I agree with that, but specs are failing and it seems to be related.

Previously, we only allowed tag names to be passed into the `tagged_with` method. This commit allows to take advantage of the interface gutentag has to offer, while being backwards compatible.
@robinboening robinboening force-pushed the extend_tagged_with_interface branch from bc42f3a to eafb1cd Compare November 18, 2021 12:53
@robinboening
Copy link
Contributor Author

I didn't notice the default value for match is any, but we expect to always match all by default. I altered the commit and force pushed. Hopefully tests will turn green now. 🤞

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Very nice! Restarted the flaky spec.

@tvdeyen tvdeyen enabled auto-merge November 18, 2021 13:16
@tvdeyen tvdeyen added this to the 6.0 milestone Nov 18, 2021
@tvdeyen tvdeyen merged commit 32c7a1e into AlchemyCMS:main Nov 18, 2021
@robinboening robinboening deleted the extend_tagged_with_interface branch November 18, 2021 20:54
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.

Allow the use of matchers in Taggable.tagged_with()
2 participants