Code review comment for lp:~suutari-olli/openlp/rename_Powerpoint_to_PowerPoint

Revision history for this message
Azaziah (suutari-olli) wrote :

Thanks for the review trb143,

however, you have clearly have missed some things and leave a few questions:
First of all, this also changes how PowerPoint is called in list of presentation
controllers. This includes both Powerpoint and Powerpoint viewer.

These are un-translatable and come directly from some part of the code which
includes “Powerpoint”. If we change “Powerpoint” from one function but leave
the other “Internals” in-touched, changes are something gets broken. If we are
going to start PowerPoint with big P, we should always bring along the 2nd the
way. While capitalizing the 1st letter seems okayish standard normally, I
think we shouldn’t do it with PowerPoint.

Translations: Really? Can’t we just backup the translations
for the current string and re-place the new one with the old ones.
(With the big P fix). This shouldn’t be too much work for one person,
if it comes to that – I’d gladly do it.

Settings migration: If something was called
“Powerpoint” it is now called: “PowerPoint”
New tests: If something was called “Powerpoint”
it is now called: “PowerPoint”

Jenkins: Please re-read the description, it clearly includes
link to Jenkins test suite. You should be able to open the other
tests from the last one easily.

As stated before, this branch only renames
Powerpoint to PowerPoint everywhere,
if the i18n strings are a problem,
I can leave them out from the branch.

What kind of tests are you perhaps hoping to see with this change?
What do you mean with setting migration?

« Back to merge proposal