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

Refactor RecoTrackSelector and RecoTrackRefSelector #9798

Merged
merged 2 commits into from
Jun 24, 2015

Conversation

cmsbuild
Copy link
Contributor

This PR refactors the commonalities of RecoTrackSelector and RecoTrackRefSelector to a new base class (RecoTrackSelectorBase; as suggested by a comment in RecoTrackRefSelector). I'm planning to add a third variant (reading tracks through View), and without the refactoring the code would become even more messy. I also took the opportunity to do a similar refactoring of the configuration parameters. I took this separate from the other developments in the hope of smoother integration for purely technical changes.

Tested in CMSSW_7_5_X_2015-06-14-2300, no changes expected in results.

@rovere @VinInn
Automatically ported from CMSSW_7_5_X #9658 (original by @makortel).

…to RecoTrackSelectorBase

I will soon add a third variant, and without the refactoring (as
suggested by a comment in RecoTrackRefSelector) the code would become
even more messy.
@cmsbuild
Copy link
Contributor Author

A new Pull Request was created by @cmsbuild for CMSSW_7_6_X.

Refactor RecoTrackSelector and RecoTrackRefSelector

It involves the following packages:

CommonTools/RecoAlgos
PhysicsTools/RecoAlgos

@cmsbuild, @cvuosalo, @vadler, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @jdolen, @makortel, @abbiendi, @mmarionncern, @jhgoh, @ahinzmann, @nhanvtran, @schoef, @mariadalfonso, @TaiSakuma 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.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@slava77
Copy link
Contributor

slava77 commented Jun 23, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor Author

@cvuosalo
Copy link
Contributor

+1

For #9798 b969345

Refactoring RecoTrackSelector and RecoTrackRefSelector code. There should be no change in run-time behavior. #9658, the 75X version of this PR, was already approved by Reco.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_7_6_X_2015-06-23-1100 show no significant differences, as expected.

@davidlange6
Copy link
Contributor

+1

1 similar comment
@monttj
Copy link
Contributor

monttj commented Jun 24, 2015

+1

@cmsbuild
Copy link
Contributor Author

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request will be automatically merged.

cmsbuild added a commit that referenced this pull request Jun 24, 2015
Refactor RecoTrackSelector and RecoTrackRefSelector
@cmsbuild cmsbuild merged commit 0fd7a83 into cms-sw:CMSSW_7_6_X Jun 24, 2015
@makortel makortel deleted the recoTrackSelectorRefactor branch October 20, 2016 11:39
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.

6 participants