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: add package example #12

Merged
merged 4 commits into from
Dec 30, 2023

Conversation

wazazaby
Copy link
Contributor

Hi!

Just a small pull request that includes a little bit of refactoring :

  • use any instead of interface{}
  • remove an useless if statement
  • refactor convertToType internals to improve readability

And most importantly a package example to demonstrate how to use gonull and what it brings to the table.

As there are no exported functions other than NewNullable (which is very straightforward and documented), I think that a package-wide example is fine for now.

I based it off the example that you wrote in the README file.

Let me know if anything bothers you.

Thank you :)

@wazazaby wazazaby marked this pull request as ready for review December 28, 2023 21:24
@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (151a4a4) 100.00% compared to head (3c96658) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #12   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           60        59    -1     
=========================================
- Hits            60        59    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@LukaGiorgadze LukaGiorgadze left a comment

Choose a reason for hiding this comment

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

Hey @wazazaby, thank you for your contribution in open-source, you are doing an amazing job!
I reviewed your code and it looks good, the only one thing is that moving example_test.go in the examples directory could be better than placing it in the main package.

Something like:
examples/json.go

So, if you could change that, I'm ready to merge.

@wazazaby
Copy link
Contributor Author

wazazaby commented Dec 30, 2023

Thank you for the review!

Agreed, the example as now been moved to an examples package :)

@LukaGiorgadze LukaGiorgadze merged commit ed7f29e into LukaGiorgadze:main Dec 30, 2023
7 checks passed
@wazazaby
Copy link
Contributor Author

@LukaGiorgadze the file should actually be named json_test.go to be ran properly, I missed it sorry for that. I will open a new PR to fix it.

@LukaGiorgadze
Copy link
Owner

LukaGiorgadze commented Dec 30, 2023

@LukaGiorgadze the file should actually be named json_test.go to be ran properly, I missed it sorry for that. I will open a new PR to fix it.

hah yeah, I merged the PR, feel free to rise a new one!
But wait, we actually don't need to run it. It's just an example for devs.

@wazazaby
Copy link
Contributor Author

Examples are usually runnable like tests and I believe it's a good thing to have them "interactive" instead of a plain text file (the README would suffice in this case).

If you want to keep them that way it's fine tho.

@LukaGiorgadze
Copy link
Owner

Examples are usually runnable like tests and I believe it's a good thing to have them "interactive" instead of a plain text file (the README would suffice in this case).

If you want to keep them that way it's fine tho.

nah, never used examples like that before, but frankly not bad for this kind of repo. Feel free to rise PR, i'll approve it 👍

@wazazaby
Copy link
Contributor Author

Great!

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.

3 participants