Fix for issue #717 #727
Fix for issue #717 #727
Conversation
…fallback for missing Event parameters.
| @@ -357,7 +357,7 @@ def display_dict(d, padding): | |||
| id = getattr(obj, obj._id_attr) | |||
| print('%s: %s' % (obj._id_attr.replace('_', '-'), id)) | |||
| if hasattr(obj, '_short_print_attr'): | |||
| value = getattr(obj, obj._short_print_attr) | |||
| value = getattr(obj, obj._short_print_attr) or 'None' | |||
There was a problem hiding this comment.
shouldn't None events just be skipped in the CLI?
target_title: None
target_title: None
target_title: None
target_title: None
target_title: None
target_title: None
target_title: None
target_title: None
target_title: None
There was a problem hiding this comment.
Actually, I don't know what these bad events are. It seems to me better to display them then to hide them. Because I consider github as a tool that helps me to show precisely what's going on in GitLab. If it hides the bad results, it may hide an important clue to a problem I'm solving.
But I don't know how other parts of github behave. It's up to you to decide what you want it to do.
There was a problem hiding this comment.
The problem I see with this issue is that there is no way to have a usable output without displaying multiple attributes. For events that would be target_type and action_name for exemple
What we could do (to be discussed):
- make the
display_dictcheck for areprmethod for the object. This method would return a key and a value to be displayed. We could use__repr__ou__unicode__for this maybe - if the method doesn't exist, fallback to the
_short_print_attrattribute - if this attribute is None raise an error so we can fix the code and add the method from 1 (not sure about this step, it's not nice for end users).
Other idea: _short_print_attr could become a tuple, and we display multiple lines when needed
|
@xarx00 could you make the commit messages a bit more meaningful? Something like: Thanks for your help on fixing the CLI 👍 |
|
@gpocentek OK, I have thought that link to the defect is enough. Also, the PEP8 test on GitHub Travis CI complains when the commit message is longer than 50 chars. |
I propose this fix for isuue #717. It fixes
event list, which didn't worked when invoked from CLI (and probably neither when used from gitlab API).