On 02/06/2012 12:29 PM, Daniel d'Andrada wrote: > Instead of shoving all parameters in the constructor. > > Signed-off-by: Daniel d'Andrada <[email protected]> > --- > include/xorg/gtest/environment.h | 64 > +++++++++++++++++++++++++++++--------- > src/Makefile.am | 6 ++-- > src/defines.h | 2 + > src/environment.cpp | 48 ++++++++++++++++++++++------ > src/main.cpp | 37 +++++++++++---------- > 5 files changed, 111 insertions(+), 46 deletions(-) > > diff --git a/include/xorg/gtest/environment.h > b/include/xorg/gtest/environment.h > index 3996507..22c82be 100644 > --- a/include/xorg/gtest/environment.h > +++ b/include/xorg/gtest/environment.h > @@ -47,16 +47,11 @@ namespace testing { > * Either associate the environment manually > * with the overall testing framework like > * @code > - * std::string xorg_conf_path("conf/dummy.conf"); > - * std::string xorg_log_file_path("/tmp/MyDummyXorg.log"); > - * int xorg_display = 133; > - * std::string server("Xorg"); > - * > - * xorg::testing::Environment* environment = new xorg::testing::Environment( > - * xorg_conf_path, > - * server, > - * xorg_display); > - * environment->set_log_file(xorg_log_file_path); > + * xorg::testing::Environment* environment = new xorg::testing::Environment; > + * environment->set_server("Xorg"); > + * environment->set_display(133); > + * environment->set_conf_file("conf/dummy.conf"); > + * environment->set_log_file("/tmp/MyDummyXorg.log"); > * testing::AddGlobalTestEnvironment(environment); > * @endcode > * or link to libxorg-gtest_main. > @@ -65,12 +60,8 @@ class Environment : public ::testing::Environment { > public: > /** > * Constructs an object to provide a global X server dummy environment. > - * @param path_to_conf Path to xserver configuration. > - * @param path_to_server Path to xserver executable. > - * @param display Display port of dummy xserver instance. > */ > - Environment(const std::string& path_to_conf, > - const std::string& path_to_server = "Xorg", int display = 133); > + Environment(); > > virtual ~Environment(); > > @@ -87,6 +78,49 @@ class Environment : public ::testing::Environment { > */ > const std::string& log_file() const; > > + /** > + * Sets the path to the desired xserver configuration file. > + * > + * It will be passed on to the xserver via the command line > + * argument "-config". > + * > + * @param path_conf_file Path to a Xorg X server .conf file. > + */ > + void set_conf_file(const std::string& path_conf_file); > + > + /** > + * Returns the path of the xserver configuration file to be used. > + * Its default value is [datadir]/xorg/gtest/dummy.conf > + * @return File path of the xserver configuration currently set > + */ > + const std::string& conf_file() const; > + > + /** > + * Sets the path to the xserver executable > + * @param path_to_server Path to a xserver executable > + */ > + void set_server(const std::string& path_to_server); > + > + /** > + * Returns the path where the xserver executable to be used. > + * Its default value is "Xorg" > + * @return Path to xserver executable. > + */ > + const std::string& server() const; > + > + /** > + * Sets the display number that the xserver will use. > + * @param diplay_num A display number. > + */ > + void set_display(int display_num); > + > + /** > + * Returns the display number of the xserver instance. > + * Its default value is 133 > + * @return Display number of the xserver. > + */ > + int display() const; > +
I prefer the format: /** * Short description * * Long description and/or details * * @param lines, etc. */ The line separating the short and long description is required due to our Doxygen setup. I ammended the commit to put the comments in this format. I also moved the default value comments to the setter functions. I think that's where most people will look to find default values. Everything else looks good! I pushed the patch to master. Thanks! -- Chase _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
