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

Add further testing to oof #27

Merged
merged 7 commits into from
May 24, 2021
Merged

Conversation

demfabris
Copy link

.

@marcospb19
Copy link
Member

Looking great, I will just request one change before merging.

oof/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Author

@demfabris demfabris left a comment

Choose a reason for hiding this comment

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

So i'm attempting to create this setup function you asked me. How would we tackle this compile error?
filter_flags needs a reference for flags_info and we cannot return them from within the function.
How would you solve this?

oof/src/lib.rs Show resolved Hide resolved
Copy link
Author

@demfabris demfabris left a comment

Choose a reason for hiding this comment

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

Added further testing. Getting weird compiler complains tho. I think it has to do with this matches macro

oof/src/lib.rs Outdated Show resolved Hide resolved
@demfabris demfabris changed the title Add unknown short flag test Add further testing to off May 21, 2021
@demfabris
Copy link
Author

So i did a little refactor in the assert approach and added test_misplaced_flag.
Let me know if i'm doing good

@demfabris demfabris changed the title Add further testing to off Add further testing to oof May 22, 2021
@marcospb19
Copy link
Member

marcospb19 commented May 22, 2021

You're doing good! If you have finished let me know it is ready for merging.

For that we usually mark the PR as draft, and then unmark when it's ready, so I'll be marking your PR as draft here.

(feel free to add more comments for any questions)

@marcospb19 marcospb19 marked this pull request as draft May 22, 2021 07:43
@demfabris
Copy link
Author

I think we're set. Every error variant is now covered.
This last one took me a while 😆

@demfabris demfabris marked this pull request as ready for review May 24, 2021 01:06
@marcospb19
Copy link
Member

It is possible that oof turns into a separated argparsing crate someday.

If you want to keep an eye on ouch, there's a button on the top right corner to receive notifications for some issues and PRs.

I created some other issues because it was a mess, if you need any mentoring again, don't be afraid to ask.

This PR is looking very good, thank you so much for the contribution.

@marcospb19 marcospb19 merged commit fee56aa into ouch-org:master May 24, 2021
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