Mir

Code review comment for lp:~josharenson/mir/command_line_config

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

~~~
6 + * Copyright © 2012 Canonical Ltd.
~~~

s/2012/2014

~~~
26 +#include "mir/options/default_configuration.h"
~~~

Foward declare instead:

namespace mir
{
namespace options
{
class DefaultConfiguration;
}
}

~~~
28 +namespace mo=mir::options;
~~~

I believe we avoid using namespace aliases in headers.

~~~
33 + -> std::shared_ptr<mo::DefaultConfiguration>;
~~~

Missing #include <memory> for shared_ptr

~~~
36 +int main(int argc, char** argv);
~~~

Not needed.

~~~
57 + * Copyright © 2012 Canonical Ltd.
~~~

s/2012/2014

~~~
74 +#include "mir/default_configuration.h"
76 +#include "mir/default_server_configuration.h"
83 +#include <memory>
~~~

^-- Not needed

~~~
87 +int argc;
88 +char const** argv;
~~~

Mark them static?

~~~
107 + [](char const* arg) { return !strncmp(arg, "--gtest_", 8); }) - argv;
~~~

Because of the use of strncmp there should be a corresponding #include <cstring>

review: Needs Fixing

« Back to merge proposal