Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DQM] Cleanup for private header usage #34773

Merged
merged 3 commits into from
Aug 10, 2021
Merged

[DQM] Cleanup for private header usage #34773

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
58cbff9
[DQM] Cleanup for private header usage
smuzaffar Aug 4, 2021
ba00dc1
move ROOTFilePB.pb.h to interface dir
smuzaffar Aug 5, 2021
a459f06
move ROOTFilePB.pb.h to interface dir
smuzaffar Aug 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion DQMOffline/Ecal/plugins/EcalZmassClient.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <memory>

// user include files
#include "DQM/Physics/src/EwkDQM.h"
#include "DQMServices/Core/interface/DQMEDHarvester.h"
#include "DQMServices/Core/interface/DQMStore.h"
#include "FWCore/Framework/interface/MakerMacros.h"
Expand Down
1 change: 0 additions & 1 deletion DQMOffline/Ecal/plugins/EcalZmassTask.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ Description: [one line class summary]

// user include files

#include "DQM/Physics/src/EwkDQM.h"
#include "DataFormats/Candidate/interface/Candidate.h"
#include "DataFormats/Common/interface/Handle.h"
#include "DataFormats/EgammaCandidates/interface/GsfElectron.h"
Expand Down
2 changes: 1 addition & 1 deletion DQMServices/Components/bin/fastHadd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ PATH=/afs/cern.ch/work/r/rovere/protocolbuf/bin
#include <thread>
#include <mutex>
#include <list>
#include "DQMServices/Core/src/ROOTFilePB.pb.h"
#include "DQMServices/Core/interface/ROOTFilePB.h"
#include <google/protobuf/io/coded_stream.h>
#include <google/protobuf/io/gzip_stream.h>
#include <google/protobuf/io/zero_copy_stream_impl.h>
Expand Down
1 change: 1 addition & 0 deletions DQMServices/Core/interface/ROOTFilePB.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#include "DQMServices/Core/src/ROOTFilePB.pb.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smuzaffar is this "trick" according to the rules? Isn't it the same as including from DQMServices/Core/src?
Why not to move directly DQMServices/Core/src/ROOTFilePB.pb.h into \interface?

Copy link
Contributor Author

@smuzaffar smuzaffar Aug 5, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes trick is according to the rules i.e. within a package one can use private headers :-) This allows unit tests of a package to use private headers and check the functionality.

I have no objections on moving ROOTFilePB.pb.h to interface too. We just need to make sure that next time when we update protobuf and re-generate these ROOTFilePB.pb.* files then the ROOTFilePB.pb.h should go in to interface.

Any objections @cms-sw/dqm-l2 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections as far as I can tell

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes trick is according to the rules i.e. within a package one can use private headers :-) This allows unit tests of a package to use private headers and check the functionality.

Yes, but when included from DQMServices/FileIO/plugins/DQMFileSaverPB.cc or DQMServices/StreamerIO/plugins/DQMProtobufReader.cc it is not "within a package" any more...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats true @perrotta but build rules only look for direct include of a private header of other package. As DQMServices/FileIO/plugins/DQMFileSaverPB.cc is not directly including src/ROOTFilePB.pb.h so it will not fail. Anyway, I will update the PR to move ROOTFilePB.pb.h in to interface

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, src/ROOTFilePB.pb.h moved to interface/ROOTFilePB.pb.h

2 changes: 1 addition & 1 deletion DQMServices/FileIO/plugins/DQMFileSaverPB.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

#include "zlib.h"
#include "DQMServices/Core/interface/DQMStore.h"
#include "DQMServices/Core/src/ROOTFilePB.pb.h"
#include "DQMServices/Core/interface/ROOTFilePB.h"
#include "FWCore/Framework/interface/LuminosityBlock.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/ServiceRegistry/interface/Service.h"
Expand Down
2 changes: 1 addition & 1 deletion DQMServices/StreamerIO/plugins/DQMProtobufReader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "FWCore/Utilities/interface/UnixSignalHandlers.h"
// #include "FWCore/Sources/interface/ProducerSourceBase.h"

#include "DQMServices/Core/src/ROOTFilePB.pb.h"
#include "DQMServices/Core/interface/ROOTFilePB.h"
#include <google/protobuf/io/coded_stream.h>
#include <google/protobuf/io/gzip_stream.h>
#include <google/protobuf/io/zero_copy_stream_impl.h>
Expand Down