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

pull request subject #10349

Closed
wants to merge 3 commits into from
Closed

pull request subject #10349

wants to merge 3 commits into from

Conversation

doanhien
Copy link

@slava77
This is the description of pull request:
Adding variable of sigma_ieta_ieta at seeding level for electron in HI reconstruction. The link to describe sigma_ieta_ieta cut is powerful to kill fake electron (which screw up the jet resolution in jet analysis) can be found here: https://twiki.cern.ch/twiki/pub/CMS/PhotonAnalyses2015/20150616.pdf. This variable was added and got PR successfully for 7_6_X. However, we would like to make a back port for 7_5_X since we will use 7_5_X for data taking in heavy ion
the number of the PR in 76X: #10194

Do I need to change the PR subject?

@doanhien
Copy link
Author

@lgray
@slava77

Is this branch name more sensible than "My" in PR of #10331?

@cmsbuild
Copy link
Contributor

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

Backport dev

It involves the following packages:

RecoEgamma/EgammaElectronProducers

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @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.
@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 Jul 24, 2015

Hi Hien,
you should enter something more sensible in the top first line when you submit the PR (you don't need to change the actual name of the branch in the github repo).
Go to the top of the pull request web page, click an "edit" button in the upper right and edit the top like that now says "Backport dev".
After that, please also edit the description of the PR (the block right below the subject line)
There should be a brief description of the change and a link to some slides or some pictures if this change introduces differences in the physics output.
If the changes are for some specialized analysis setup, it needs to be mentioned.

Thank you.

@slava77
Copy link
Contributor

slava77 commented Jul 24, 2015

BTW, at this point the branch has merge conflicts.

@doanhien doanhien changed the title Backport dev Adding variable of sigma_ieta_ieta at seeding level for electron in HI reconstruction. Jul 24, 2015
@doanhien doanhien changed the title Adding variable of sigma_ieta_ieta at seeding level for electron in HI reconstruction. Adding variable of sigma_ieta_ieta at seeding level for electron in HI reconstruction. The link to describe sigma_ieta_ieta cut is powerful to kill fake electron (which screw up the jet resolution in jet analysis) can be found here: https://twiki.cern.ch/twiki/pub/CMS/PhotonAnalyses2015/20150616.pdf. This variable was added and got PR successfully for 7_6_X. However, we would like to make a back port for 7_5_X since we will use 7_5_X for data taking in heavy ion. Jul 24, 2015
@slava77
Copy link
Contributor

slava77 commented Jul 24, 2015

So, I was asking for something really brief in the top one-line "pull request subject" and a longer description in the larger available block right below it (the latter still has a somewhat non-descript "backport to 7_5_X", while the former contains more than about a line of text.
Please correct.

... beyond this cosmetics, the pull request was submitted to a wrong branch (76X as it stands now).
This one should be closed and the 75X version created. [I'm deriving that this is actually for 75X from the message "backport to 7_5_X")

@doanhien doanhien changed the title Adding variable of sigma_ieta_ieta at seeding level for electron in HI reconstruction. The link to describe sigma_ieta_ieta cut is powerful to kill fake electron (which screw up the jet resolution in jet analysis) can be found here: https://twiki.cern.ch/twiki/pub/CMS/PhotonAnalyses2015/20150616.pdf. This variable was added and got PR successfully for 7_6_X. However, we would like to make a back port for 7_5_X since we will use 7_5_X for data taking in heavy ion. pull request subject Jul 26, 2015
@doanhien
Copy link
Author

@slava77
Hi,

I put the description here. Is it correct?
Adding variable of sigma_ieta_ieta at seeding level for electron in HI reconstruction. The link to describe sigma_ieta_ieta cut is powerful to kill fake electron (which screw up the jet resolution in jet analysis) can be found here: https://twiki.cern.ch/twiki/pub/CMS/PhotonAnalyses2015/20150616.pdf. This variable was added and got PR successfully for 7_6_X. However, we would like to make a back port for 7_5_X since we will use 7_5_X for data taking in heavy ion

@slava77
Copy link
Contributor

slava77 commented Jul 26, 2015

Hi Hien,

Thank you for the details. It's a start.
Please provide the number of the PR in 76X as well.

The PR subject (the top line) is now " pull request subject ". It should be changed.
The longer description should go right below it (it now says "backport to 7_5_X"). There is a pen-like edit button in every post that you make on github.

@slava77
Copy link
Contributor

slava77 commented Jul 27, 2015

-1

Thank you for updating the PR description.

now, what remains is

  • change the PR subject from "pull request subject" to something descriptive
  • submit this PR appropriately to 75X (it was made to 76X, which is the reason for the merge conflicts)

@davidlange6
Copy link
Contributor

i agree with @slava77's plan - so I'll close this

@doanhien
Copy link
Author

@slava77

Hi Slava,

Can I change this PR to 75X here or I have to resubmit this with 75X?

Thanks,

@slava77
Copy link
Contributor

slava77 commented Jul 27, 2015

@doanhien
target branch reassignment is not possible. Please update #10331 or create a new PR

@lgray
Copy link
Contributor

lgray commented Jul 30, 2015

@doanhien Can you please click the "close pull request" button at the bottom of the webpage? :-)

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