build: restore configure formatting, add final message for warnings#623
Uh oh!
There was an error while loading. Please reload this page.
Conversation
silverwindforce-pushed the configure-warnings branch from 80ac5c7 to c7c1656Comparesilverwind changed the title 
rvagg commented Jan 27, 2015
fine by me, can you give us a screenshot or fixed-format paste into here? |
silverwind commented Jan 27, 2015
Here you go, had to reduce the font size a bit to fit everything on this tiny screen.
|
rvagg commented Jan 27, 2015
lgtm but I'd like signoff from @bnoordhuis too cause I know he did some refactoring of #483 in #585 and may also have suggestions for wording (I don't really like "Careful! Warnings happened" but can't think of a good alternative). |
silverwind commented Jan 28, 2015
I'm open to any suggestions regardind the print and its color. Also noticed I could refactor the final check by removing the msg variable, if that style is prefered. |
configure Outdated
There was a problem hiding this comment.
I believe it's slightly more pythonic to replace the global with:
defwarn(msg): # ...warn.warned=Truewarn.warned=FalseThere was a problem hiding this comment.
Good call, I'll fix that.
silverwindforce-pushed the configure-warnings branch 3 times, most recently from 857a4b4 to d2723a2Comparesilverwind commented Jan 28, 2015
@bnoordhuis updated with your suggestions. |
silverwind commented Jan 28, 2015
@bnoordhuis wait, I'll push another update as I noticed this when no warnings occured: |
configure Outdated
There was a problem hiding this comment.
You need to define warn.warned, else you get an AttributeError below when warn() hasn't been called.
On the topic of the commit log, can you make the first line fit in <= 50 characters? Thanks!
silverwindforce-pushed the configure-warnings branch from 075a85a to 668bde8Comparesilverwind commented Jan 28, 2015
Whoops, looks like I've broken this PR. Won't let me re-open it while the branch is still active. Guess I need to open a new one, sorry. |
3 participants
This is a follow-up on #483. It fixes the readability concern of #595 by restoring the options print to its previous pretty-printed form while adding a final print in case if warnings happened.
I also took the liberty to change the color of warnings from light red to light yellow, as I think that color is more appropriate and red should be reserved for errors only.