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

SLHAs from Shih-Chuan #8007

Closed
wants to merge 1 commit into from

Conversation

dnash86
Copy link
Contributor

@dnash86 dnash86 commented Mar 1, 2015

SLHA files received from Shih-Chuan for Summer12 EXO request

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2015

A new Pull Request was created by @dnash86 for CMSSW_5_3_X.

SLHAs from Shih-Chuan

It involves the following packages:

Configuration/Generator

@vciulli, @covarell, @thuer, @cmsbuild, @nclopezo, @bendavid can you please review it and eventually sign? Thanks.
@ghellwig, @Martin-Grunewald 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.
@smuzaffar you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@bendavid
Copy link
Contributor

bendavid commented Mar 2, 2015

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2015

This pull request is fully signed and it will be integrated in one of the next CMSSW_5_3_X IBs unless changes or unless it breaks tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar, @ktf

@bendavid
Copy link
Contributor

bendavid commented Mar 2, 2015

@davidlange6 , the mechanism to inline the slha tables in the fragments was not backported to pythia6 nor to 53x (and in addition 53x predates the migration of these to externals), so I think they're needed directly in the release as such.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2015

This pull request is fully signed and it will be integrated in one of the next CMSSW_5_3_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar, @ktf

@davidlange6
Copy link
Contributor

@Degano , @nclopezo - could you add these to the data externals? (There should already be a package for these) Thanks!

@ghost
Copy link

ghost commented Mar 20, 2015

hold
I'm working on importing these files through cms-data repository and cmsdist.

@ghost
Copy link

ghost commented Mar 20, 2015

@davidlange6 @dnash86 Done, the PR in cmsdist is here: cms-sw/cmsdist#1457. Please check that the cms-data repository containing those files matches the files that would have gone in this PR. The version has been done at this commit: cms-data/Configuration-Generator@88f6b60

@bendavid
Copy link
Contributor

Uh, I'm confused. I thought the migration to external data package was only done in the 7x release series, and for 53x we were still adding these files directly to the release.

@davidlange6
Copy link
Contributor

@bendavid- not sure - there is no real advantage to doing so aside from enabling a quick patch release, which is not the case here.

@bendavid
Copy link
Contributor

Hi,
I'm REALLY confused at what's happened here.
It seems to me that not only were the 61 new files added to the externals, but also ALL the data files in Configuration/Generator/data were added to the external (and then they were NOT removed from the cmssw release in the 53x branch, as it was done in the 7x series)

So it seems to me that triggered by 61 extra files, we have backported a large change from 7x to 53x without discussing it, and without doing it consistently.
So actually my preference would be to revert cms-sw/cmsdist#1457 and just merge this pull request. But ok let's discuss it at the ORP in 30 mins...

@ghost
Copy link

ghost commented Mar 25, 2015

@bendavid @davidlange6 I've created and merged a new PR for cmsdist that points to those 61 files only: cms-sw/cmsdist#1461.

@davidlange6
Copy link
Contributor

Thanks @Degano. I'll close this PR then as it should not be needed.

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