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

CMSSW 7 0 X switch to consumes API #808

Merged

Conversation

mileva
Copy link
Contributor

@mileva mileva commented Sep 12, 2013

Code modified to use the consumes API. getByLabel() invokations are replaced with getByToken().

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mileva for CMSSW_7_0_X.

CMSSW 7 0 X switch to consumes API

It involves the following packages:

Validation/MuonRPCDigis

@rovere, @deguio can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.

@nclopezo
Copy link
Contributor

Hi,

I ran the usual tests on top of CMSSW_7_0_X_2013-09-12-0200, all tests passed:

https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc481/477/console

you can see the artifacts here:

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/477

@deguio
Copy link
Contributor

deguio commented Sep 13, 2013

ciao @mileva thanks for the changes.
I was wondering if you could also take care of moving the histogram booking in the beginRun. I see that it is done in the beginJob.
The reason why this is needed has been discussed back in June. see:
https://indico.cern.ch/getFile.py/access?contribId=3&resId=0&materialId=slides&confId=256775

thank you in advance,
F.

@rovere @danduggan

@cmsbuild
Copy link
Contributor

Pull request #808 was updated. @danduggan, @rovere, @deguio can you please check and sign again.

# Name of the root file which will contain the histos
outputFile = cms.untracked.string('')
outputFile = cms.untracked.string('output.root')
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason why you left this one?

@deguio
Copy link
Contributor

deguio commented Sep 18, 2013

@mileva I would just replace it with an empty string as it was before.
thanks,
F.

@rovere

@mileva
Copy link
Contributor Author

mileva commented Sep 18, 2013

OK,
leave the empty string.
Thanks!
Roumyana


From: deguio [notifications@github.com]
Sent: 18 September 2013 13:44
To: cms-sw/cmssw
Cc: Roumyana Mileva Hadjiiska
Subject: Re: [cmssw] CMSSW 7 0 X switch to consumes API (#808)

@milevahttps://github.com/mileva I would just replace it with an empty string as it was before.
thanks,
F.

@roverehttps://github.com/rovere


Reply to this email directly or view it on GitHubhttps://github.com//pull/808#issuecomment-24658162.

@deguio
Copy link
Contributor

deguio commented Sep 18, 2013

@mileva can you implement the change and commit the code again please?
thanks in advance,
F.

@cmsbuild
Copy link
Contributor

Pull request #808 was updated. @danduggan, @nclopezo, @rovere, @deguio can you please check and sign again.

edm::Handle<RPCDigiCollection> rpcDigis;
event.getByLabel(digiLabel, rpcDigis);
// Obsolete code commented and replaced with getByToken invokes (labels and proccesses are specified on variable init) - stanislav
// event.getByLabel("g4SimHits", "MuonRPCHits", simHit);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not comment old code. Just remove it.

@cmsbuild
Copy link
Contributor

Pull request #808 was updated. @danduggan, @nclopezo, @rovere, @deguio can you please check and sign again.

@deguio
Copy link
Contributor

deguio commented Sep 19, 2013

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it?

ktf added a commit that referenced this pull request Sep 20, 2013
…yToken

Consumes API --  Validation/MuonRPCDigis
@ktf ktf merged commit 54719dd into cms-sw:CMSSW_7_0_X Sep 20, 2013
@deguio
Copy link
Contributor

deguio commented Sep 20, 2013

see the discussion in #807
@mileva @eliasron

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

5 participants