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

Test template filters #333

Merged
merged 3 commits into from Oct 3, 2020
Merged

Conversation

orsinium
Copy link
Contributor

@orsinium orsinium commented Oct 2, 2020

  1. Make template filters kind of type-safe. leave type reflection to the template engine and accept already the right type. Also, it helps to correctly match the number of arguments, making error messages in that case user-friendly.
  2. Add unit tests for happy-path for custom filters.
  3. Do not suppress errors from filters, making them propagate up into the template engine. The engine gracefully handles it and returns as the second argument from Template.Execute method indicating template execution failure.

Thank you for the amazing project! I'm migrating from IFTTT :)

@coveralls
Copy link

Coverage Status

Coverage increased (+2.6%) to 71.515% when pulling 3cb789a on orsinium-forks:improve-templates into d22455f on muesli:master.

@orsinium
Copy link
Contributor Author

orsinium commented Oct 2, 2020

CI fails not because of my changes, I'm sure. Is it flaky? 🤔

@rocket357
Copy link

rocket357 commented Oct 2, 2020 via email

@muesli
Copy link
Owner

muesli commented Oct 2, 2020

Just a heads up: @penguwin is already working on fixing the Windows builds.

@muesli muesli merged commit 42fa791 into muesli:master Oct 3, 2020
@muesli
Copy link
Owner

muesli commented Oct 3, 2020

Great contribution, thank you!

@orsinium orsinium deleted the improve-templates branch October 5, 2020 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants