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.

Sign up for GitHub

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

Jump to bottom

Issue9185 #9248

Closed
wants to merge 17 commits into from
Closed

Issue9185 #9248

wants to merge 17 commits into from

Conversation

njskalski
Copy link
Contributor

I have to write tests yet, but I wanted to see full Travis CI output.

Fixes #9185

Review on Reviewable

@highfive
Copy link

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 11, 2016
@njskalski njskalski mentioned this pull request Jan 11, 2016
Closed
@Manishearth
Copy link
Member

r? @Ms2ger

@jdm
Copy link
Member

jdm commented Jan 11, 2016

@bors-servo: try

@bors-servo
Copy link
Contributor

⌛ Trying commit 86e1030 with merge d374876...

bors-servo pushed a commit that referenced this pull request Jan 11, 2016
Auto merge of #9248 - askalski:issue9185, r=<try>
d374876 
Issue9185

I have to write tests yet, but I wanted to see full Travis CI output.

Fixes #9185

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9248)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Jan 11, 2016
@KiChjang
Copy link
Contributor

@bors-servo retry

  • SSL snafu

@bors-servo
Copy link
Contributor

⌛ Trying commit 86e1030 with merge f6cc563...

bors-servo pushed a commit that referenced this pull request Jan 11, 2016
Auto merge of #9248 - askalski:issue9185, r=<try>
f6cc563 
Issue9185

I have to write tests yet, but I wanted to see full Travis CI output.

Fixes #9185

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9248)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Jan 11, 2016
@jdm
Copy link
Member

jdm commented Jan 11, 2016

@bors-servo: retry

  • crates.io was down

@bors-servo
Copy link
Contributor

⌛ Trying commit 86e1030 with merge 3dc2e7c...

bors-servo pushed a commit that referenced this pull request Jan 11, 2016
Auto merge of #9248 - askalski:issue9185, r=<try>
3dc2e7c 
Issue9185

I have to write tests yet, but I wanted to see full Travis CI output.

Fixes #9185

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9248)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

@njskalski
Copy link
Contributor Author

Wow, I didn't know about "try". So far so good. Now I need to add tests. Can someone suggest me what kind of tests? wpt, or unit-tests?

@KiChjang
Copy link
Contributor

These changes are affecting the HTML script element, so you'll need WPT tests.

Also, "try" is a privileged command that only reviewers or people with "try" access can use.

@jdm
Copy link
Member

jdm commented Jan 11, 2016

That being said, you're welcome to add yourself to that list: https://github.com/servo/saltfs/blob/master/homu/cfg.toml#L83

@KiChjang
Copy link
Contributor

You can just retry it until it passes. But since you're just doing a "try" run, all you really care about is whether WPT tests relevant to your PR are failing, and looking at the results, they aren't.

@njskalski
Copy link
Contributor Author

@bors-servo: retry

@bors-servo
Copy link
Contributor

⌛ Trying commit fe69ead with merge ecd1b4d...

bors-servo pushed a commit that referenced this pull request Jan 16, 2016
Auto merge of #9248 - askalski:issue9185, r=<try>
ecd1b4d 
Issue9185

I have to write tests yet, but I wanted to see full Travis CI output.

Fixes #9185

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9248)
<!-- Reviewable:end -->
@njskalski njskalski mentioned this pull request Jan 16, 2016
Closed
@nox
Copy link
Contributor

nox commented Jan 17, 2016

@bors-servo try-

@nox
Copy link
Contributor

nox commented Jan 17, 2016

@bors-servo r- clean force

@Manishearth
Copy link
Member

@bors-servo r- clean try- force

@Manishearth Manishearth reopened this Jan 17, 2016
@Manishearth
Copy link
Member

@bors-servo clean retry

@KiChjang
Copy link
Contributor

@askalski Can you try git commit --amend and then git push -f on your branch? That should fix homu.

@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Jan 27, 2016
@njskalski
Copy link
Contributor Author

@KiChjang yeah, you are right!

@nox nox added S-needs-squash Some (or all) of the commits in the PR should be combined. S-needs-rebase There are merge conflict errors. labels Feb 19, 2016
@nox
Copy link
Contributor

nox commented Feb 19, 2016

The commits need to be rebased to remove that local merge, and then they need to be squashed together.

@nox
Copy link
Contributor

nox commented Feb 19, 2016

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 784f01e with merge a8bbba2...

bors-servo pushed a commit that referenced this pull request Feb 19, 2016
Auto merge of #9248 - askalski:issue9185, r=<try>
a8bbba2 
Issue9185

I have to write tests yet, but I wanted to see full Travis CI output.

Fixes #9185

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9248)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Feb 19, 2016
@KiChjang
Copy link
Contributor

KiChjang commented Mar 3, 2016

@askalski Is this still being worked on?

@njskalski
Copy link
Contributor Author

I was waiting for review to know what to fix.

@KiChjang
Copy link
Contributor

KiChjang commented Mar 3, 2016

I believe @nox gave a reply to rebase and squash?

@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 3, 2016

I seem to have lost track of this PR; apologies for that. I'll try to review tomorrow.

@jdm
Copy link
Member

jdm commented Mar 10, 2016

Tomorrow came and went...

@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 18, 2016

Reviewed and addressed some issues at #10079. Thanks for your contribution, and again my apologies for dropping the ball here.

@Ms2ger Ms2ger closed this Mar 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

Labels
S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. S-needs-squash Some (or all) of the commits in the PR should be combined. S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@njskalski @highfive @Manishearth @jdm @bors-servo @KiChjang @eefriedman @larsbergstrom @nox @Ms2ger