-
Notifications
You must be signed in to change notification settings - Fork 100
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 informative comments #196
base: master
Are you sure you want to change the base?
Add informative comments #196
Conversation
Pull Request Test Coverage Report for Build 833
💛 - Coveralls |
Any clue what these 'checks' are complaining about? All I did was newline some code and add comments - and coverage dropped 5%. I'm considering getting these for my own repos, but if this behavior isn't configurable, that's a big minus. |
Sorry for not getting to this sooner, and about the weird coverage issue. I've noticed that Coveralls can sometimes be finicky about combining coverage files from multiple Travis jobs. Clearly this PR isn't dropping any coverage, so we can ignore that. The Travis check is complaining about Black formatting, which I do want to adhere to. If you've installed Black, you can follow the (admittedly lacking) CONTRIBUTING "Getting Started" section to fix the formatting. If you'd rather not install Black, the Travis job log does specify which lines should be reformatted and how. Once we're all set on the formatting, reviewing will be easier, and we can merge this. I think the biggest Black formatting issues are probably that it wants two spaces before an inline comment, and it doesn't like the two consecutive octothorpes. Thanks for your work! This is a great improvement! |
Codecov Report
@@ Coverage Diff @@
## master #196 +/- ##
=======================================
Coverage 94.61% 94.61%
=======================================
Files 46 46
Lines 4955 4955
=======================================
Hits 4688 4688
Misses 267 267 Continue to review full report at Codecov.
|
@HunterMcGushion Consistency on cost of customizability - fair enough; glad your config supports additional whitespaces for vertical alignment purposes - other repos yelled about it. Also, this PR page gives me a sensation of walking into a robo factory where you're approached by autonomous assistants upon attempting a change to the repo. (Not necessarily bad - in fact, rather unique for me) |
I seem to have broke it even more by editing as suggested - I'll let you take it from here; don't have to merge my PR, it was intended as a demo anyway. Thanks for the response |
@OverLordGoldDragon, yeah, I'm a big fan of keeping the codebase consistent. Unfortunately, in this case, it's led to some headaches, and I apologize for that.
I certainly understand the feeling! Despite their odd behavior from time to time (now), I find that the robots are worth the trouble and usually end up saving me from shooting myself in the foot.
Understood! Thanks for taking the time to make the PR and give me some feedback. I'm sorry again for the strange behavior of the robot assistants. I'll try to cherry-pick your commit or something to ensure your name stays attached to the work! |
Looks like the reason everything was breaking is because a new Keras version was released (2.3.0), which Travis was using automatically. Our previous, passing builds were using 2.2.5. |
@HunterMcGushion Thanks for notifying - a bit off-topic, but I'm configuring my own Travis, and would like to ignore multiple error codes; barely know anything about Linux, so using
fails. Any suggestion or tutorial link as to how to accomplish this? Couldn't find much in Travis docs. Thanks ahead |
Yeah Travis was pretty hard for me to get started, too. Since this isn't related to the HH issue, are you available on Slack? I'd be happy to try to help you via Direct Message. You can find me on the HH Slack channel |
No description provided.