-
Notifications
You must be signed in to change notification settings - Fork 176
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 test for optional route parameter #43
Add test for optional route parameter #43
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use const/let
over var
!!
It does not add anything to the set of tests but it can add to attracts attention to optional routes.
Sure. Both styles were in the code already, so @3imed-jaberi how would you feel about me doing a sweep and fixing it everywhere in this file? |
@smarts no worries - they didn't mean to alienate in any way, @3imed-jaberi is excited and happy to see your contributions! 🎉 🎉 |
Interesting, this test passes now. Looks like the bug has been fixed. |
thanks @smarts |
Happy to help. Thanks to you @niftylettuce and @3imed-jaberi as well! |
@smarts, sorry ... I didn't mean to insult or belittle or anything negative ... I just tried to provide a comment summarizing the situation in this PR and I wanted to say thank you for your contribution and that your work provides a good example .. |
Test for #40