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

Don't use ExUnit in production code #20

Merged
merged 3 commits into from
Feb 8, 2024
Merged

Don't use ExUnit in production code #20

merged 3 commits into from
Feb 8, 2024

Conversation

tshakah
Copy link
Contributor

@tshakah tshakah commented Feb 6, 2024

This changes the use of assert to raise on nil, and removes the commented out asserts

I'm actually not sure what tests to write to get this to fail

This changes the use of `assert` to raise on `nil`, and
removes the commented out asserts
@g-andrade
Copy link
Collaborator

g-andrade commented Feb 6, 2024

It's OK, I think there's no need to test it since the check is only there to ensure that, in the rare event that the caller of this module is handling it wrong, a more obscure exception won't be produced in later stages of decoding - it is indeed an assertion.

Now, for some reason CI isn't being triggered... I'm going to close and reopen the PR to see if that does it.

@g-andrade g-andrade closed this Feb 6, 2024
@g-andrade g-andrade reopened this Feb 6, 2024
@g-andrade g-andrade closed this Feb 6, 2024
@g-andrade g-andrade reopened this Feb 6, 2024
@tshakah
Copy link
Contributor Author

tshakah commented Feb 6, 2024

Github has been playing up for me today, although their status page says nothing is wrong

@g-andrade
Copy link
Collaborator

Ah, CI is failing because of the infamous test coverage threshold, which I can never get to 100 because of Elixir structs... we just need to set it in mix.exs to whatever value is now failing with.

@tshakah
Copy link
Contributor Author

tshakah commented Feb 6, 2024

You can get structs to be covered by using them in pattern matches in tests, I'll push a commit with an example

@tshakah tshakah closed this Feb 6, 2024
@tshakah tshakah reopened this Feb 6, 2024
@tshakah
Copy link
Contributor Author

tshakah commented Feb 6, 2024

Those two commits have gotten me another 1% coverage locally

@tshakah tshakah closed this Feb 6, 2024
@tshakah tshakah reopened this Feb 6, 2024
@g-andrade
Copy link
Collaborator

You can get structs to be covered by using them in pattern matches in tests, I'll push a commit with an example

Ohh, thanks for the tip! And for your contribution as well. I’ll merge it and tag a new version tomorrow if I get a chance; if not, later this week for sure.

@g-andrade
Copy link
Collaborator

Closes #19

@g-andrade g-andrade closed this Feb 8, 2024
@g-andrade g-andrade reopened this Feb 8, 2024
@g-andrade g-andrade merged commit 1ef2531 into sqids:main Feb 8, 2024
15 checks passed
@g-andrade
Copy link
Collaborator

0.1.3 tagged and published to Hex 🎉

@tshakah
Copy link
Contributor Author

tshakah commented Feb 8, 2024

Thanks 🎉

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.

2 participants