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

Remove CaloGeometryRecord caching #11414

Closed
wants to merge 82 commits into from

Conversation

argiro
Copy link
Contributor

@argiro argiro commented Sep 22, 2015

Fix a rare crash in clustering

rovere and others added 30 commits August 5, 2015 11:11
…that they can be tuned independently. Fix order of python import logic. No regression expected.
…minosityBlocks> is needed to compile (in analogy to PoolOutputModule)

*streamer output module now also needs beginLumi and endLumi handlers
…ext LS is signalled to input source

*correct initialization of variable ls
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @argiro for CMSSW_7_4_12_patchX.

Remove CaloGeometryRecord caching

It involves the following packages:

RecoParticleFlow/PFClusterProducer

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @bachtis, @lgray this is something you requested to watch as well.
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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.

Note that this branch is designed for requested bug fixes specific to the CMSSW_7_4_12 release.
If you wish to make a pull request for the CMSSW_7_4_X release cycle, please use the CMSSW_7_4_X branch instead

@argiro argiro mentioned this pull request Sep 22, 2015
@cvuosalo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@lgray
Copy link
Contributor

lgray commented Sep 22, 2015

@cvuosalo unless something had changed tests don't run against the patchesX branch.

@slava77
Copy link
Contributor

slava77 commented Sep 23, 2015

@argiro
Stefano
the fix should be done based on a recent 7_4_12_patch release (or, even the 7_4_12_patchX branch itself)

@argiro
Copy link
Contributor Author

argiro commented Sep 23, 2015

On 23 Sep 2015, at 05:59, Slava Krutelyov notifications@github.com wrote:

@argiro
Stefano
the fix should be done based on a recent 7_4_12_patch release (or, even the 7_4_12_patchX branch itself)

I did it based on 7_4_12_patch4, what is wrong exactly ?


Reply to this email directly or view it on GitHub.

@lgray
Copy link
Contributor

lgray commented Sep 23, 2015

Hi Stefano, the problem is that you used the same branch to make the CMSSW
7_4_X PR and the 7_4_patchX PR, and they are somewhat diverged at the
moment.

I would recommend that you take down the 7_4_X, rebase this branch and
update it using the commands I showed you yesterday except using
CMSSW_7_4_12_patchX instead of CMSSW_7_4_X, and then make a new branch that
you rebase to 7_4_X and then submit a new 7_4_X PR.

On Wed, Sep 23, 2015 at 9:22 AM, argiro notifications@github.com wrote:

On 23 Sep 2015, at 05:59, Slava Krutelyov notifications@github.com
wrote:

@argiro
Stefano
the fix should be done based on a recent 7_4_12_patch release (or, even
the 7_4_12_patchX branch itself)

I did it based on 7_4_12_patch4, what is wrong exactly ?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#11414 (comment).

@argiro
Copy link
Contributor Author

argiro commented Sep 23, 2015

so #11414 is ok, but #11415 is not, correct ?

On 23 Sep 2015, at 09:44, Lindsey Gray notifications@github.com wrote:

Hi Stefano, the problem is that you used the same branch to make the CMSSW
7_4_X PR and the 7_4_patchX PR, and they are somewhat diverged at the
moment.

I would recommend that you take down the 7_4_X, rebase this branch and
update it using the commands I showed you yesterday except using
CMSSW_7_4_12_patchX instead of CMSSW_7_4_X, and then make a new branch that
you rebase to 7_4_X and then submit a new 7_4_X PR.

On Wed, Sep 23, 2015 at 9:22 AM, argiro notifications@github.com wrote:

On 23 Sep 2015, at 05:59, Slava Krutelyov notifications@github.com
wrote:

@argiro
Stefano
the fix should be done based on a recent 7_4_12_patch release (or, even
the 7_4_12_patchX branch itself)

I did it based on 7_4_12_patch4, what is wrong exactly ?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#11414 (comment).


Reply to this email directly or view it on GitHub.

@lgray
Copy link
Contributor

lgray commented Sep 23, 2015

Right now 11414 is based on 7_4_X, since it shared the same branch as 11415.
You can see in 11415 that there is only one commit being brought in by the
PR, that's what we want to achieve for 11414.
However, it means you have to take down one of them and make a new PR based
on a new branch with the right history, since right now both PRs come from
the same branch.

On Wed, Sep 23, 2015 at 10:34 AM, argiro notifications@github.com wrote:

so #11414 is ok, but #11415 is not, correct ?

On 23 Sep 2015, at 09:44, Lindsey Gray notifications@github.com wrote:

Hi Stefano, the problem is that you used the same branch to make the
CMSSW
7_4_X PR and the 7_4_patchX PR, and they are somewhat diverged at the
moment.

I would recommend that you take down the 7_4_X, rebase this branch and
update it using the commands I showed you yesterday except using
CMSSW_7_4_12_patchX instead of CMSSW_7_4_X, and then make a new branch
that
you rebase to 7_4_X and then submit a new 7_4_X PR.

On Wed, Sep 23, 2015 at 9:22 AM, argiro notifications@github.com
wrote:

On 23 Sep 2015, at 05:59, Slava Krutelyov notifications@github.com
wrote:

@argiro
Stefano
the fix should be done based on a recent 7_4_12_patch release (or,
even
the 7_4_12_patchX branch itself)

I did it based on 7_4_12_patch4, what is wrong exactly ?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#11414 (comment).


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#11414 (comment).

@argiro
Copy link
Contributor Author

argiro commented Sep 23, 2015

closing and rebasing

@argiro argiro closed this Sep 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment