Code review comment for lp://staging/~nick-dedekind/qtmir/miral-DisplayConfigurationStorage

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> +++ src/platforms/mirserver/miral/edid.cpp
> + std::ostringstream stringStream;
> + stringStream << "Incorrect EDID structure size {" << data.size() <<
> "}";
> + throw std::runtime_error(stringStream.str());
>
> This would do:
> throw std::runtime_error(std::string("Incorrect EDID structure size:") +
> std::to_string(data.size()));
>
>
> ++ src/platforms/mirserver/miral/persist_display_config.cpp
> since you remove the #if MIR_SERVER_VERSION block, please merge the list of
> mir headers so it is neater.
>
> +++ src/platforms/mirserver/miral/persist_display_config.cpp
> Nitpick: space after the "for":
> + for(auto iter = output.modes.cbegin(); iter != output.modes.cend(); ++iter,
> i++) {
>
>
> +++ src/platforms/mirserver/qmirserver_p.cpp
> Nitpick: please add "// namespace" after the anonymous namespace closing brace
>
>
> Rest looks good

Fixed.

« Back to merge proposal