None
@ssahani, could you explain this change please?
This function in original python 2 script has an optional parameter: path This function in python 3 script has one required positional parameter: path
You completely change function specification.
Before function could be called in 3 different ways:
With this change you forbid to use methods 1 and 2.
I asked you to revert your changes for the same place at https://pagure.io/standard-test-roles/pull-request/265#_1,34
Please check : commits history: https://pagure.io/standard-test-roles/commits/master
If I marge your PR what commit comment will look like? It will be:
Merge 270 Port to python3: another set
Port to python3: another set
Please change this comment to appropriate meaningful form, that will help read git history.
get_artifact_path() get_artifact_path(path="/path") get_artifact_path("/path")
Let's see the usage of the function how this is used.
'log_file = get_artifact_path(LOG_FILE) ' Now where you plan to use it with a black value. option 2 and 3 still works.
Please check : commits history: https://pagure.io/standard-test-roles/commits/master If I marge your PR what commit comment will look like? It will be: Merge 270 Port to python3: another set Please change this comment to appropriate meaningful form, that will help read git history.
you sure Note: it contains 3 PRS but yes should be properly should be named
@ssahani, you have a very clear task: "port scripts to Python 3". Please go and read your Jira ticket.
In this PR you have changed function specification. This is unrelated to Python3 porting. In your PR comment you say: "This is about python3 porting". But, it is not.
I am trying to review your PR according to your PR comment. Please explain, how this change to function specification is related to Python3 porting?
@bgoncalv @mvadkert @bookwar please review this RP.
@ssahani , please write comments to your PR in readable format.
Above comment very hard to read and understand. Please add punctuation marks. Is this statement or question? Please save our time.
Please don't try to make me understand what is my task.
your function specification does not have any use case.
If I marge your PR what commit comment will look like? It will be: it's not marge it's merge. Please fix your spelling first
If I marge your PR what commit comment will look like? It will be: it's not marge it's merge. Please fix your spelling first You are right, sorry.
You are right, sorry.
@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.
rebased onto 4b7ced6e1e2581a1d52d889212cd260bfd7ec08b
updated
lgtm
Looks good, merge. Thank you.
Commit f196ed6f fixes this pull-request
Pull-Request has been merged by astepano
None