-
Notifications
You must be signed in to change notification settings - Fork 5
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 match preview tab 2 #9
Conversation
at first travis failed due to problem with compilation. Maybe it cannot find new lib you introduced. |
|
||
int s, nscripts; | ||
|
||
pattern = StringValueCStr(str_pattern); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not simple char *pattern = StringValueCStr(str_pattern);
premature initialization is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- char *pattern = StringValueCStr(str_pattern); ok
- You mean premature initialization of res_hash? I wanted to return hash (empty or not) in any case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean char * one. For res_hash it make sense to init it to empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok.
expect(are_installed [family]).to be true | ||
end | ||
|
||
it "returns a installed family for serif alias" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is duplicite statement, maybe typo or missing word?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy&paste I would say :)
All suggestions except for three of them are implemented. |
No so far. |
OK, it would be nice to fix it, but it is not required. |
otherwise it looks good. |
Thanks! |
No description provided.