-
Notifications
You must be signed in to change notification settings - Fork 280
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 support for port and env #1395
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.
Thanks
Now the question is: once this PR will be reviewed by other maintainers and it will about to get merged. Should you add separate commits to this PR to start using this method ? Or should a new PR need to be added to start using it? It's an open question. I'm fine with both |
Hey @ccoVeille I think we can add commits in this PR only and make changes to test methods using the ports |
I was also thinking it's the way to do, as it would be the best way to see if the code provided will work for the tests where it could be used. But I preferred to ask 😅 |
Thank you for simplifying everything. The tests file way simpler. I'm glad you worked on this |
Thank you @ccoVeille. Please do let me know if there should be anymore changes/improvements or if i have overseen or missed out on anything else. |
merging remote to local
This PR will conflict with mine #1426 And it's OK. Let's merge this one, mine is in draft, and I can easily resolve everything in my PR |
Pull Request Template
Description:
Breaking Changes (if applicable):
Checklist:
goimport
andgolangci-lint
.Thank you for your contribution!