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

Update SiStripDetSummary usage in the SiStrip Payload Inspector #20448

Merged

Conversation

pieterdavid
Copy link
Contributor

This should solve the conflict between the changes in #20274 and #20184, and fix the compile errors in new PR tests and the integration builds.
@mmusich could you check that this does not break the SiStrip Payload Inspector?

using the StandaloneTrackerTopology solution as in other classes
defined here
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2017

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2017

A new Pull Request was created by @pieterdavid (Pieter David) for master.

It involves the following packages:

CondCore/SiStripPlugins

@ggovi, @cmsbuild can you please review it and eventually sign? Thanks.
@ghellwig, @mmusich, @VinInn this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2017

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20448/605

@mmusich
Copy link
Contributor

mmusich commented Sep 10, 2017

Hi @pieterdavid, thanks for the quick fix. It looks OK to me. I cannot test this until tomorrow. For the time being I suggest this to be merged as is, to avoid compilation issues in other integration tests, and then test it and eventually correct afterward. Unfortunately, I think, the PI applications cannot be tested in cmssw unit tests if (at least reading) database credentials are made available to the bot. @ggovi please comment on the last point.

@davidlange6
Copy link
Contributor

davidlange6 commented Sep 10, 2017 via email

@cmsbuild cmsbuild merged commit 4b929e5 into cms-sw:master Sep 10, 2017
@mmusich
Copy link
Contributor

mmusich commented Sep 11, 2017

Hello all,
just for the sake of completeness I tested these additions in CMSSW_9_4_X_2017-09-10-2300 which contains this PR, and things look good - as expected:

  • SiStripApvGainsByPartition
    sistripapvgainsbypartition

  • SiStripApvGainsComparatorByPartition

sistripapvgainscomparatorbypartition

  • SiStripBadStripByRegion

sistripbadstripbyregion

  • SiStripDetVOffByRegion

sistripdetvoffbyregion

therefore there is no need to further amend these codes.

M.

@pieterdavid pieterdavid deleted the fix-sistripdetsummary-payloadinspector branch September 28, 2018 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants