Hi, ssahani, could you please do not change any logic in re-factoring? Would be you so kind to re-vert this change?
Please re-vert, you made here endless recursion. _signal_handler calls self.
rebased onto 06b7cd20564d80015ae681701b71d3d256318521
updated
please re-vert =""
please remove
Why ?
It's no readable at all . There should be a space ending a function.
Please send a new separate PR for code reformatting. What this PR is about? Fixing formatting or porting to Python3? Please simplify review task for us. Each PR - small logical change.
PS Personally I do not like blank lines inside function. But, this is arguable personal preference. https://www.python.org/dev/peps/pep-0008/#blank-lines https://visualgit.readthedocs.io/en/latest/pages/code_style.html#blank-lines https://softwareengineering.stackexchange.com/questions/251216/is-too-much-whitespace-a-bad-thing
I don't agree with you. Is just simple stuff does not need any separate PR. What is so difficult just for spaces you need a separate PR
Again pasting a bunch of links from internet how it's helping?
@bgoncalv @mvadkert please review.
why was this change needed?
I think this blank line makes more clear that the function finished and the next commands are not part of it, but any decision you make here is fine by me.
@ssahani could you just revert the change of get_artifact_path ? It has nothing to do with python3 porting. A part from it everything else looks good.
@bgoncalv OK will do . Going to open a another PR for this
rebased onto b51efcc5d6c0f72423928ada9676cedff5672da3
updated.
indentation changed, this should not be part of _signal_handler
rebased onto a12571886a3483a051470a8010c80810c2a134fd
right updated thanks !
lgtm
@ssahani, could you please explain what was reason for this change? https://docs.python.org/3/library/subprocess.html#subprocess.call I see that Python3 does support : subprocess.call
subprocess.call
No nothing to do with python3. check_call verifies if there is error. Would make another PR.
rebased onto 1bc1980d68f3067322f9f0d8092f239def3bd9e1
rebased onto 6cc71cda5808d753390139f7cd2ec36538cbda72
Commit 5f36250e fixes this pull-request
Pull-Request has been merged by astepano