-
Notifications
You must be signed in to change notification settings - Fork 110
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
ENH: Capture hints from Git #4338
Conversation
This has a potential side effect: the custom result summary renderer of get will now be executed.
Oh, I broke things... but with the merged WTF warning it seems...
|
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.
quick minor comments from just scanning the diff but not thinking about the overall change
datalad/support/gitrepo.py
Outdated
@@ -2504,6 +2506,9 @@ def _fetch_push_helper( | |||
pi = info_cls._from_line(line) | |||
if add_remote: | |||
pi['remote'] = remote | |||
# There were errors, but Git provided hints | |||
if 'error' in pi['operations']: | |||
pi['hints'] = hints if hints is not '' else None |
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.
is
should be avoided because you're comparing a string literal. (IIRC Python actually started warning about this in a very recent version, perhaps 3.8.)
You could use ==
, but in this case I think hints or None
would suffice.
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. To be honest, I don't know how/why this (hints or None
) works. What can I google to find out about this?
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.
Hmm, I first checked pydoc or
. Ignoring the BNF notation and skipping down to the "x or y" paragraph, I think that description is pretty good:
The expression "x or y" first evaluates *x*; if *x* is true, its value
is returned; otherwise, *y* is evaluated and the resulting value is
returned.
So, x if x else y
but slightly shorter (and more easily parsed, depending on the eyes).
Searching online for "python boolean operations", I had to scroll past a few tutorials, but not too far down saw a link to this part of the python docs, which says the same thing as the pydoc or
page, perhaps more clearly.
Codecov Report
@@ Coverage Diff @@
## master #4338 +/- ##
==========================================
+ Coverage 88.85% 88.91% +0.05%
==========================================
Files 285 285
Lines 37491 37553 +62
==========================================
+ Hits 33313 33390 +77
+ Misses 4178 4163 -15
Continue to review full report at Codecov.
|
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.
Cool, thanks! I believe the crippled FS test failure is unrelated.
Sometimes, Git has "hints" why pushing fails.
For example, if I attempt a push without integrating remote changes, it says
datalad push
swallowed these hints. In this PR I'm attempting to catch them and report all hints at the end of the result rendering.A few examples:
A situation with lots of the same hints
And with no hints:
side effects:
We found out that the custom result summary renderer of get is never executed, because it would previously only be called if
result_renderer
was set totailored
. With b8b282d, it should be called now.