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.

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

Duplication of parameter declarations in SiStrip tracker DIGI #23634

Closed
fabiocos opened this issue Jun 21, 2018 · 5 comments
Closed

Duplication of parameter declarations in SiStrip tracker DIGI #23634

fabiocos opened this issue Jun 21, 2018 · 5 comments

Comments

@fabiocos
Copy link
Contributor

During the review of #23621 I commented about the evident duplication of parameters between

https://cmssdt.cern.ch/lxr/source/SimGeneral/MixingModule/python/SiStripSimParameters_cfi.py
and
https://cmssdt.cern.ch/lxr/source/SimTracker/SiStripDigitizer/python/SiStripDigiSimLink_cfi.py

It would seem appropriate to have a common PSet definition that is then imported in both configurations, avoiding duplications that are error prone. The problem was not introduced by the mentioned PR, it just enlarged it, and given the 10_2_X deadline I preferred to move forward anyway. But I open this issue to keep track of the request of code cleaning and follow it

@cmsbuild
Copy link
Contributor

A new Issue was created by @fabiocos Fabio Cossutti.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@fabiocos
Copy link
Contributor Author

assign simulation

@cmsbuild
Copy link
Contributor

New categories assigned: simulation

@mdhildreth,@civanch you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

It looks to me that the SiStripDigiSimLink_cfi.py and the DigiSimLinkProducer it refers to are both completely unused, so easiest would be to just remove them.

@fabiocos
Copy link
Contributor Author

fabiocos commented Aug 2, 2018

After the merge of #23644 this issue can be closed

@fabiocos fabiocos closed this as completed Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants