feat(exec) Filter by tags in exec cmd#344
Conversation
|
Looks good! Can you also add tests for this? I'm trying not to merge anything new without tests, and sometimes it's hard cause no foundation exists. Let me know if I can help you out with this if it's too much! |
|
the test for this function is already here: https://github.com/knqyf263/pet/blame/main/snippet/snippet_test.go#L307 |
|
Looked into this a bit more, and I want to e2e test the filtering functionality not just this one function, but it's taking some time. I think it's a slightly big refactor so not going to lay it on you 😄 My goal from adding these additional tests is just making sure the old functionality doesn't change with this change, but in an automated way. Sorry to keep you waiting! Will merge this soon. |
|
Sorry for the wait - this took longer than expected and I got busy. Haven't forgotten about it! Just want to add a proper test foundation for all commands. This one uses quite some things behind the scenes (ex. filtering using fish) so testing is hard, still figuring that out. One thing that's blocking me from merging this right away is the fact that the flags changed, that's not backwards compatible. Not a big deal probably but if someone is using pet in a script we'll break it for them. |
Issue #, if available:
Description of changes:
Reuse of the FilterByTags method and adding the ability to filter by tags, not just a single tag, when using the 'exec' command.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.