-
Notifications
You must be signed in to change notification settings - Fork 1
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
Thread2 #2
Conversation
There's still a problem with I don't think it's necessary to call |
Finally, please check all places where you set In fact, you only have to set |
Finally, I would recommend to rearrange the code a bit: This might sound pedantic, but you will thank me when you have to read this code again in 1 year :-) |
done
done
I fail to understand what this is about. You mean a better ordering of the functions? |
yes |
@Spacechild1 I think this is roughly ready to merge, bump to 0.3.2 and do a Deken release. What do you think? |
flite.c
Outdated
|
||
if (!x->x_wave) { | ||
const char *voxstring; | ||
voxstring = vox->s_name; |
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 really nitpicky, but why do you have the assignment on a seperate line? This should be simply const char *voxstring = vox->s_name
(I guess this comes from a misunderstanding about C. With C89 you had to declare any local variables at the top of the block, which often lead to a split between declaration and assignment. With C99 local variables can be declared anywhere.)
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.
you don't have to change it, it's more meant as a comment
Looks good to me! I haven't really tested it in practice, but I guess you did :-) |
Total thanks @Spacechild1 :) v0.3.2 is up on Deken. |
Improvements to the threading part