Skip to content
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

Refactored python function to create command line arguments #342

Merged
merged 1 commit into from Apr 27, 2016

Conversation

@ashrwin
Copy link
Contributor

ashrwin commented Apr 27, 2016

This commit is for issue #338


This change is Reviewable

@@ -115,19 +115,17 @@ def make_step(self, command):
])


def make_cd_string(command):
return "cd /c/buildbot/slave/windows/build ; " + command
def make_cd_command(command):

This comment has been minimized.

@aneeshusa

aneeshusa Apr 27, 2016

Member

This should be renamed to express the intent of the function, instead of what it is doing. Let's use make_win_command, since this function takes a regular command and makes it palatable to Windows.

def make_cd_string(command):
return "cd /c/buildbot/slave/windows/build ; " + command
def make_cd_command(command):
return ["bash", "-l", "-c", "cd /c/buildbot/slave/windows/build ; " + command]

This comment has been minimized.

@aneeshusa

aneeshusa Apr 27, 2016

Member

You'll need to split this onto two lines to fix the lint error (see the Travis logs for details). Also, the space between build and the semicolon can be removed.

I'd suggest using one line to construct the cd-prefixed string, and the second line to return the whole array.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 27, 2016

This LGTM; please squash these commits together (you can use the commit message from the first commit).

@ashrwin ashrwin force-pushed the ashrwin:ash-dev branch from d3d3797 to 04a0052 Apr 27, 2016
@aneeshusa
Copy link
Member

aneeshusa commented Apr 27, 2016

Thank you for the PR!
@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Apr 27, 2016

📌 Commit 04a0052 has been approved by aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Apr 27, 2016

Testing commit 04a0052 with merge dcb93ca...

bors-servo added a commit that referenced this pull request Apr 27, 2016
Refactored python function to create command line arguments

This commit is for issue #338

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/342)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 27, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 04a0052 into servo:master Apr 27, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@ashrwin ashrwin deleted the ashrwin:ash-dev branch Apr 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.