| Message ID | 20211005073114.3997303-6-hiroh@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Tue, Oct 05, 2021 at 04:31:13PM +0900, Hirokazu Honda wrote: > "using namespace" in a header file propagates the namespace to > the files including the header file. So it should be avoided. > This removes "using namespace" in header files in lc-compliance. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > src/lc-compliance/environment.cpp | 2 +- > src/lc-compliance/environment.h | 8 +++----- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/src/lc-compliance/environment.cpp b/src/lc-compliance/environment.cpp > index 9e24b5e3..f4583c8e 100644 > --- a/src/lc-compliance/environment.cpp > +++ b/src/lc-compliance/environment.cpp > @@ -13,7 +13,7 @@ Environment *Environment::get() > return &instance; > } > > -void Environment::setup(CameraManager *cm, std::string cameraId) > +void Environment::setup(libcamera::CameraManager *cm, std::string cameraId) How about adding 'using namespace libcamera' in this file for consistency ? I can change this when applying if desired. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > { > cm_ = cm; > cameraId_ = cameraId; > diff --git a/src/lc-compliance/environment.h b/src/lc-compliance/environment.h > index 1c7d9a55..ba308732 100644 > --- a/src/lc-compliance/environment.h > +++ b/src/lc-compliance/environment.h > @@ -9,23 +9,21 @@ > > #include <libcamera/libcamera.h> > > -using namespace libcamera; > - > class Environment > { > public: > static Environment *get(); > > - void setup(CameraManager *cm, std::string cameraId); > + void setup(libcamera::CameraManager *cm, std::string cameraId); > > const std::string &cameraId() const { return cameraId_; } > - CameraManager *cm() const { return cm_; } > + libcamera::CameraManager *cm() const { return cm_; } > > private: > Environment() = default; > > std::string cameraId_; > - CameraManager *cm_; > + libcamera::CameraManager *cm_; > }; > > #endif /* __LC_COMPLIANCE_ENVIRONMENT_H__ */
Hi Laurent, On Tue, Oct 5, 2021 at 6:45 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Hiro, > > Thank you for the patch. > > On Tue, Oct 05, 2021 at 04:31:13PM +0900, Hirokazu Honda wrote: > > "using namespace" in a header file propagates the namespace to > > the files including the header file. So it should be avoided. > > This removes "using namespace" in header files in lc-compliance. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > --- > > src/lc-compliance/environment.cpp | 2 +- > > src/lc-compliance/environment.h | 8 +++----- > > 2 files changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/src/lc-compliance/environment.cpp b/src/lc-compliance/environment.cpp > > index 9e24b5e3..f4583c8e 100644 > > --- a/src/lc-compliance/environment.cpp > > +++ b/src/lc-compliance/environment.cpp > > @@ -13,7 +13,7 @@ Environment *Environment::get() > > return &instance; > > } > > > > -void Environment::setup(CameraManager *cm, std::string cameraId) > > +void Environment::setup(libcamera::CameraManager *cm, std::string cameraId) > > How about adding 'using namespace libcamera' in this file for > consistency ? I can change this when applying if desired. > I did so because this file is small enough. Please feel free to do so if you prefer. Thanks, -Hiro > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > { > > cm_ = cm; > > cameraId_ = cameraId; > > diff --git a/src/lc-compliance/environment.h b/src/lc-compliance/environment.h > > index 1c7d9a55..ba308732 100644 > > --- a/src/lc-compliance/environment.h > > +++ b/src/lc-compliance/environment.h > > @@ -9,23 +9,21 @@ > > > > #include <libcamera/libcamera.h> > > > > -using namespace libcamera; > > - > > class Environment > > { > > public: > > static Environment *get(); > > > > - void setup(CameraManager *cm, std::string cameraId); > > + void setup(libcamera::CameraManager *cm, std::string cameraId); > > > > const std::string &cameraId() const { return cameraId_; } > > - CameraManager *cm() const { return cm_; } > > + libcamera::CameraManager *cm() const { return cm_; } > > > > private: > > Environment() = default; > > > > std::string cameraId_; > > - CameraManager *cm_; > > + libcamera::CameraManager *cm_; > > }; > > > > #endif /* __LC_COMPLIANCE_ENVIRONMENT_H__ */ > > -- > Regards, > > Laurent Pinchart
diff --git a/src/lc-compliance/environment.cpp b/src/lc-compliance/environment.cpp index 9e24b5e3..f4583c8e 100644 --- a/src/lc-compliance/environment.cpp +++ b/src/lc-compliance/environment.cpp @@ -13,7 +13,7 @@ Environment *Environment::get() return &instance; } -void Environment::setup(CameraManager *cm, std::string cameraId) +void Environment::setup(libcamera::CameraManager *cm, std::string cameraId) { cm_ = cm; cameraId_ = cameraId; diff --git a/src/lc-compliance/environment.h b/src/lc-compliance/environment.h index 1c7d9a55..ba308732 100644 --- a/src/lc-compliance/environment.h +++ b/src/lc-compliance/environment.h @@ -9,23 +9,21 @@ #include <libcamera/libcamera.h> -using namespace libcamera; - class Environment { public: static Environment *get(); - void setup(CameraManager *cm, std::string cameraId); + void setup(libcamera::CameraManager *cm, std::string cameraId); const std::string &cameraId() const { return cameraId_; } - CameraManager *cm() const { return cm_; } + libcamera::CameraManager *cm() const { return cm_; } private: Environment() = default; std::string cameraId_; - CameraManager *cm_; + libcamera::CameraManager *cm_; }; #endif /* __LC_COMPLIANCE_ENVIRONMENT_H__ */
"using namespace" in a header file propagates the namespace to the files including the header file. So it should be avoided. This removes "using namespace" in header files in lc-compliance. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- src/lc-compliance/environment.cpp | 2 +- src/lc-compliance/environment.h | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-)