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

Add 'vertical-align' #27

Merged
merged 1 commit into from Aug 20, 2013
Merged

Add 'vertical-align' #27

merged 1 commit into from Aug 20, 2013

Conversation

@june0cho
Copy link
Contributor

june0cho commented Aug 18, 2013

No description provided.


impl CssVerticalAlignValue {
pub fn new(type_: css_vertical_align_e, length: css_fixed, unit: css_unit) -> CssVerticalAlignValue {
if type_ == CSS_VERTICAL_ALIGN_INHERIT {

This comment has been minimized.

@metajack

metajack Aug 19, 2013

Contributor

This needs to be a match. It will eliminate the final else case and the compiler will enforce that all cases are handled.

@metajack
Copy link
Contributor

metajack commented Aug 19, 2013

Looks good aside from needing to change if to match. I'll merge once that change is made.

+ use 'match' instead of 'if'
@june0cho
Copy link
Contributor Author

june0cho commented Aug 20, 2013

@metajack Thanks for the comment. I changed if to match. The default case( _ => XXX ) still exists since type css_vertical_align_e is c_enum.

@metajack

This comment has been minimized.

Copy link

metajack commented on bb3b55b Aug 20, 2013

r+

metajack added a commit that referenced this pull request Aug 20, 2013
@metajack metajack merged commit a55ab9e into servo:master Aug 20, 2013
@june0cho june0cho deleted the june0cho:vertical-align branch Aug 30, 2013
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

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