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

Fix Vagrantfile after #374 #394

Merged
merged 3 commits into from
Jul 12, 2016

Conversation

aneeshusa
Copy link
Contributor

@aneeshusa aneeshusa commented Jun 8, 2016

This is the second time I've broken it, so also add a test.


This change is Reviewable

@aneeshusa aneeshusa force-pushed the fix-Vagrantfile-after-374 branch 2 times, most recently from 6f9dcfd to 895859a Compare June 8, 2016 08:10
@aneeshusa
Copy link
Contributor Author

r? @larsbergstrom or @edunham

I haven't had a chance to figure out how to make the smoke test pass on Travis. However, we've gotten a large influx of new contributors that are trying to use the Vagrantfile and are running into the fact that it's broken, so I think it's important it gets fixed ASAP. (See here and here for examples.) I've updated this to include just the fix for now so we can make it easier for new contributors; the test can be added later.

@aneeshusa
Copy link
Contributor Author

Caught another contributor at #434.

@edunham
Copy link
Contributor

edunham commented Jul 8, 2016

Reviewed 2 of 2 files at r1.
Review status: 1 of 6 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #420) made this pull request unmergeable. Please resolve the merge conflicts.

@edunham
Copy link
Contributor

edunham commented Jul 11, 2016

@bors-servo r+

@aneeshusa I think you're good to go on this once it's up to date.

@bors-servo
Copy link
Contributor

📌 Commit 98c983d has been approved by edunham

@bors-servo
Copy link
Contributor

🔒 Merge conflict

Environment variables are now passed as a list,
instead of a string.
Originally, I restricted these scripts to pure sh to avoid too many
compatibility issues between Linux and OS X, but I've changed my mind
and now prefer using bash to gain a few nice features.
@aneeshusa
Copy link
Contributor Author

@bors-servo r=edunham
Merge conflict was very small (1-2 lines).

@bors-servo
Copy link
Contributor

📌 Commit 1a1eeb2 has been approved by edunham

@bors-servo
Copy link
Contributor

⌛ Testing commit 1a1eeb2 with merge 6295fde...

bors-servo pushed a commit that referenced this pull request Jul 12, 2016
Fix Vagrantfile after #374

This is the second time I've broken it, so also add a test.

<!-- 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/394)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - travis

@bors-servo bors-servo merged commit 1a1eeb2 into servo:master Jul 12, 2016
This was referenced Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants