-
Notifications
You must be signed in to change notification settings - Fork 8
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 new advance checker: blank-import #45
Add new advance checker: blank-import #45
Conversation
Hi @Antonboom , please kindly review the PR Also I have some questions that I think you can give inputs:
output from
thank you so much! |
hi @Antonboom have you got any chance to look at the PR? thankyou 🙇 |
@hendrywiranto, hi! the end of the year is time of Christmas holidays, sorry 🙂 ☃️ 🎄 |
Okay Anton, sorry for nudging and enjoy your holiday ! 🎅 |
Pull Request Test Coverage Report for Build 7629719802
💛 - Coveralls |
@hendrywiranto, hi! Please review changes from me.
Because no such packages in
|
7ab4272
to
7664183
Compare
The change looks good! made me wonder if it's better to rename the checker into also thanks for checking for the unit test, but I still can't run it successfully from local with the same error 😢 |
Great idea, could I ask you to do renaming? Thanks.
Please, try
directly, without P.S. |
But from other side we have https://github.com/Antonboom/testifylint/blob/master/CONTRIBUTING.md#inefficient-assert. And looks more consistent to have
instead of
🤷♂️ 🤔 😭 |
Lol sorry this is a typo on my end, didn't realize I typed black there 😆
I see, just realized that.. Then I think it's okay to keep the existing name 👍 |
I tried that too, still kinda the same minimum details
I'm using ubuntu 20.04 via WSL. |
Thanks for you job, but will be more attentive 👍 |
@Antonboom thanks for the work too ! 👍 |
Summary
Add new advance checker:
blank-import
Motivation
As stated in the issue:
Related issues
Closes #37
Closes #38
Testing
https://github.com/gophersbd/ormpb/blob/master/doc.go
https://github.com/Scalingo/grafana-scalingo