----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121400/#review71865 -----------------------------------------------------------
This needs a unit test comparing generated output with what you expect. You'll want to cover a few different argument sets - in particular, the different ways of getting the version information. See the tests/ directory for inspiration, and let me know if you get stuck with that. modules/ECMCreateVersionResource.cmake <https://git.reviewboard.kde.org/r/121400/#comment50089> From your previous reply, it seemed like all these are optional. You may not want to mark PRODUCT_NAME optional, say, on the basis it doesn't make much sense not to include it, but I would mark COMPANY_NAME and COPYRIGHT optional at least. You should document below what happens when an argument is omitted (and empty string is put into the file, which should be ignored by most tools). modules/ECMCreateVersionResource.cmake <https://git.reviewboard.kde.org/r/121400/#comment50088> You should note that <version> is expected to be in the form x.y.z, where x, y and z are all numbers. You may want to allow x.y.z.w as well (the CMake project() command also allows that form, but ecm_setup_version doesn't), but you'll need to change your code to account for it. modules/ECMCreateVersionResource.cmake <https://git.reviewboard.kde.org/r/121400/#comment50087> Why not: string(REGEX REPLACE "^([0-9]+)\.([0-9]+)\.([0-9]+).*" "\1,\2,\3,0" RC_VERSION_COMMAS "${RC_VERSION}") ? - Alex Merry On Dec. 10, 2014, 11:57 p.m., Nicolás Alvarez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121400/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2014, 11:57 p.m.) > > > Review request for Build System and KDE Frameworks. > > > Repository: extra-cmake-modules > > > Description > ------- > > Windows has a way for executables and shared libraries (DLLs) to contain > version information in a machine-readable format. This information can be > seen in Explorer in the file properties (http://goo.gl/UbJGte) and also in > the tooltip when hovering the file. It's also used by Windows Installer to > avoid overwriting an existing DLL with an older version. > > Apart from the version number, this resource contains information like the > "product name", "file description", "company name", and "copyright". While > optional, if we're going to add a version resource, we might as well fill > these in properly; why not? :) > > This change adds a CMake macro to generate a Windows resource script with > version information passed to the macro. > > The macro documentation is a single line, obviously needs work. > > > Diffs > ----- > > modules/ECMVersionResource.rc.in PRE-CREATION > modules/ECMCreateVersionResource.cmake PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/121400/diff/ > > > Testing > ------- > > Tested with CMake 3.0.2 in kcoreaddons (review /r/121401), MSVC2013, Windows > 7. Explorer tooltip shows description and version. File properties shows all > the new information. > > > Thanks, > > Nicolás Alvarez > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
