Update espeak-ng to 1.51-dev commit cad1c8e8#12280
Conversation
Adds readme for updating espeak submodule, sourced from: https://github.com/nvaccess/nvda/wiki/Updating-eSpeak-NG
This comment has been minimized.
This comment has been minimized.
michaelDCurran
left a comment
There was a problem hiding this comment.
I'm okay with the update instructions to be included, but I disagree with it being in include. I think it should be in devDocs or another directory, otherwise it looks a bit too important for the general user of the repository and may be misleading.
|
I did consider putting it in devDocs, but I wanted to make it easy to find when trying to update the espeak submodule. I figured proximity would help. |
|
I also hoped the naming of the file |
| "phonemelist.c", | ||
| "readclause.c", | ||
| "setlengths.c", | ||
| "soundIcon.c", |
There was a problem hiding this comment.
how did you know to add this here?
There was a problem hiding this comment.
The updating-espeak.md should cover.. or at least hint at this. The source espeak project is built from Makefile.am, I diffed this file (last commit we used vs new commit).
There was a problem hiding this comment.
I could clarify that in the md file?
There was a problem hiding this comment.
Ah right so it is only if the build fails?
I was expecting this in the "doing the update"
There was a problem hiding this comment.
Ah right so it is only if the build fails?
Good point, I guess the answer is "it depends". The build may not always fail, we explicitly exclude certain modules of espeak that we don't use. However this is only possible because of the way that these modules are implemented. I'd say it is a good idea to always do that diff and make a decision about whether changes need to be made.
Link to issue number:
None
Summary of the issue:
Several issues in espeak have been fixed, espeak devs have asked for feedback.
Description of how this pull request fixes the issue:
Updates espeak to commit cad1c8e8
Also adds a readme for updating espeak submodule, sourced from:
https://github.com/nvaccess/nvda/wiki/Updating-eSpeak-NG
Testing strategy:
Build and run locally
Known issues with pull request:
The added readme duplicates the instructions found on the wiki: https://github.com/nvaccess/nvda/wiki/Updating-eSpeak-NG
I think it is more helpful to have these in the repository, if get agreement on this, I'll update the wiki to refer to this readme.
Change log entry:
See changes file.
Code Review Checklist:
This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.
Mark items you have considered by checking them.
You can do this when editing the Pull request description with an x:
[ ]becomes[x].You can also check the checkboxes after the PR is created.