D21305: Add the FreeBSD default-path for os-release.

[Harald Sitter]

sitter added a comment.


  I do wonder if we couldn't just move the freebsd path to the end of the list and drop the ifdef. As far as linux is concerned we still obey the lookup order but simply have an additional path where we may look (and where the file should not ever exist on linux). Conversely on the freebsd side the file should never exist in the other two paths so it would simply fall through to usr/local.
  
  That said, the code is fine, ifdefs just trip me up when reading ^^

INLINE COMMENTS

> kosrelease.cpp:79
>  {
> -    if (QFile::exists(QStringLiteral("/etc/os-release"))) {
> -        return QStringLiteral("/etc/os-release");
> -    } else if (QFile::exists(QStringLiteral("/usr/lib/os-release"))) {
> -        return QStringLiteral("/usr/lib/os-release");
> -    } else {
> -        return QString();
> +    for (const auto& path : { 
> +#ifdef Q_OS_FREEBSD

& goes on the left of the space please

> kosrelease.cpp:81
> +#ifdef Q_OS_FREEBSD
> +        QStringLiteral("/usr/local/etc/os-release"),
> +#endif

The indentation is missing a space on every line. Currently it aligns with the bracket when it fact it should align with the arguments after the opening bracket.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D21305

To: adridg, sitter
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190520/91cc744c/attachment.html>