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

EXO Val Dev backported to 72X #5715

Merged
merged 4 commits into from Oct 8, 2014

Conversation

perrotta
Copy link
Contributor

@perrotta perrotta commented Oct 7, 2014

The idea is to backport #5320 (originally from @ndaci, and merged in 73X since two weeks already) in 72X.
This PR would allow monitoring also in 72X a set of paths from Exotica included in the Run2 HLT menu.
Of course, I let @deguio and the release managers to decide about it. But I think it would be a pity not being able of monitoring those HLT paths in 72X, when the PR exists in 73X and it is now ready also for 72X.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 7, 2014

A new Pull Request was created by @perrotta for CMSSW_7_2_X.

EXO Val Dev backported to 72X

It involves the following packages:

HLTriggerOffline/Exotica

@nclopezo, @danduggan, @rovere, @cmsbuild, @deguio, @ojeda 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.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@deguio
Copy link
Contributor

deguio commented Oct 7, 2014

+1
I don't see a problem. we can discuss at the ORP today.
ciao,
F.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 7, 2014

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

@davidlange6
Copy link
Contributor

why is this a pointer (that is then leaked, it seems)
std::map<unsigned int, int> * countobjects = new std::map<unsigned int, int>;

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 7, 2014

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

@perrotta
Copy link
Contributor Author

perrotta commented Oct 7, 2014

Yes. @davidlange6: I agree with you that there would have been a leakage in the parts of code you pointed out. Let me notice that such a leakage was already there in the version of the file currently in the release, before this PR.
With the last commit I provide a possible fix for it. If @ndaci agrees with it, it can also be forward ported to 73X.

@ndaci
Copy link
Contributor

ndaci commented Oct 7, 2014

Many thanks for providing the fix!!
Yes I agree, please keep this fix in both 72X and 73X :)

Great!
Nadir

2014-10-07 12:28 GMT+02:00 perrotta notifications@github.com:

Yes. @davidlange6 https://github.com/davidlange6: I agree with you that
there would have been a leakage in the parts of code you pointed out. Let
me notice that such a leakage was already there in the version of the file
currently in the release, before this PR.
With the last commit I provide a possible fix for it. If @ndaci
https://github.com/ndaci agrees with it, it can also be forward ported
to 73X.


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

@deguio
Copy link
Contributor

deguio commented Oct 7, 2014

+1
thanks for fixing this,
F.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 7, 2014

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

davidlange6 added a commit that referenced this pull request Oct 8, 2014
@davidlange6 davidlange6 merged commit 42eb2df into cms-sw:CMSSW_7_2_X Oct 8, 2014
@perrotta perrotta deleted the ExoValDevBackportedTo72X branch October 25, 2014 09:47
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