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

Fix issue #692 #696

Merged
merged 3 commits into from
Nov 21, 2017
Merged

Fix issue #692 #696

merged 3 commits into from
Nov 21, 2017

Conversation

emlun
Copy link
Member

@emlun emlun commented Nov 20, 2017 •
edited by pr-preview bot

See #692

Preview | Diff

@emlun emlun added this to the WD-07 milestone Nov 20, 2017
@emlun emlun mentioned this pull request Nov 20, 2017
Closed
jcjones
jcjones approved these changes Nov 20, 2017
View reviewed changes
Copy link
Contributor

@jcjones jcjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely an improvement.

@jcjones
Copy link
Contributor

jcjones commented Nov 20, 2017

Note: I'm not sure what the Travis build error is talking about.

$ bikeshed spec
LINE 65: Line doesn't match the grammar k:v; k:v; k:v:
<!--line count correction 0-->
ERR Did not generate, due to fatal errors

pr-preview compiled it fine, so it's probably fine to merge.

@equalsJeffH
Copy link
Contributor

equalsJeffH commented Nov 20, 2017 •
edited

@jcjones

I'm not sure what the Travis build error is talking about.

LINE 65: Line doesn't match the grammar k:v; k:v; k:v:

yeah, I figured that out in order to get PR #693 to build (see f728b91) -- basically, one cannot (presently) have comments in <pre> blocks. i spose we oughta submit a bikeshed issue if no one else has as yet. (done: speced/bikeshed#1139)

equalsJeffH
equalsJeffH approved these changes Nov 20, 2017
View reviewed changes
Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@equalsJeffH
Copy link
Contributor

review: #696 (review)), LGTM, thanks!

@nadalin
Copy link
Contributor

nadalin commented Nov 20, 2017

@akshayku @AngeloKai @christiaanbrand Please review

akshayku
akshayku approved these changes Nov 20, 2017
View reviewed changes
Copy link
Contributor

@akshayku akshayku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@nadalin
Copy link
Contributor

nadalin commented Nov 20, 2017

@emlun once @christiaanbrand approves please merge

christiaanbrand
christiaanbrand approved these changes Nov 21, 2017
View reviewed changes
Copy link

@christiaanbrand christiaanbrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@emlun emlun merged commit 311d50f into master Nov 21, 2017
@emlun emlun deleted the issue-692 branch November 21, 2017 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@equalsJeffH equalsJeffH equalsJeffH approved these changes

@jcjones jcjones jcjones approved these changes

@christiaanbrand christiaanbrand christiaanbrand approved these changes

@akshayku akshayku akshayku approved these changes

@AngeloKai AngeloKai Awaiting requested review from AngeloKai

Assignees

@emlun emlun

Labels
stat:breaking subtype:algorithms/WebIDL type:technical
Projects
None yet
Milestone
WD-07
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@emlun @jcjones @equalsJeffH @nadalin @christiaanbrand @akshayku