Manage PluginFactory plugins with unique_ptr in SimMuon #25900
Conversation
This works for PluginFactory returning either raw pointer or unique_ptr.
makortel
changed the title
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25900/8371
|
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: SimMuon/DTDigitizer @cmsbuild, @civanch, @mdhildreth, @kpedro88 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild, please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
| @@ -27,7 +27,8 @@ namespace CLHEP { | |||
| } | |||
|
|
|||
| GEMDigiProducer::GEMDigiProducer(const edm::ParameterSet& ps) | |||
| : digiModelString_(ps.getParameter<std::string>("digiModelString")) | |||
| : gemDigiModel_{GEMDigiModelFactory::get()->create("GEM" + ps.getParameter<std::string>("digiModelString") + "Model", | |||
| ps)} | |||
| { | |||
There was a problem hiding this comment.
@makortel , do you not think that complex initialisation within constructor may be difficult for code reader? I understand that this code now is more effective than previous variant, however, may be enough simple removal of digiModelString_ class member?
There was a problem hiding this comment.
I don't think replacing digiModelString_ with ps.getParameter<std::string>("digiModelString") would make the code much harder to read. Although my main motivation for this construct was to avoid additional std::unique_ptr<GEMDigiModel>{GEMDigiModelFactory::get()...} that would be needed for the assignment in the constructor body.
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25900/8393
|
|
Pull request #25900 was updated. @cmsbuild, @civanch, @mdhildreth, @kpedro88 can you please check and sign again. |
|
@cmsbuild, please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+upgrade |
|
+1 |
cmsbuild
added
fully-signed
simulation-approved
and removed
pending-signatures
simulation-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
Also some further cleanup. This PR is preparatory work to change the PluginFactory to return a
std::unique_ptr.Tested in CMSSW_10_5_X_2019-02-05-1100 , no changes expected.