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

Moved code to new FWCore/Reflection package #27287

Merged
merged 3 commits into from Jun 24, 2019

Conversation

Dr15Jones
Copy link
Contributor

PR description:

Moved code dealing with run-time understanding of a class to its
own package.
This also removes the ROOT dependency from FWCore/Utilities.

PR validation:

Used git grep for all the file names to find which ones to change. The changes compile and link.

Moved code dealing with run-time understanding of a class to its
own package.
This also removes the ROOT dependency from FWCore/Utilities.
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27287/10495

  • This PR adds an extra 328KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@Dr15Jones
Copy link
Contributor Author

The requested code formats are not for any lines modified by this pull request. The changes in PhysicsTools/CondLiteIO are part of #27278. The one change to DataFormats/PatCandidates/src/UserData.cc does not appear to be in any presently created pull request.

@Dr15Jones
Copy link
Contributor Author

@fabiocos how do you want to proceed with the code-formatting?

@fabiocos
Copy link
Contributor

@Dr15Jones #27278 overlaps with both this PR and #26592 by @wddgit :

Testing PR #  27278  for branch  master
File  CondCore/ESSources/plugins/CondDBESSource.cc  modified in PRs #  ['26592', '27278']
File  CondCore/ESSources/plugins/CondDBESSource.h  modified in PRs #  ['26592', '27278']
File  PhysicsTools/CondLiteIO/plugins/FWLiteESRecordWriterAnalyzer.cc  modified in PRs #  ['26592', '27287', '27278']
File  PhysicsTools/CondLiteIO/plugins/FWLiteESSource.cc  modified in PRs #  ['26592', '27278']
File  PhysicsTools/CondLiteIO/src/RecordWriter.cc  modified in PRs #  ['27287', '27278']

I am ready to integrate it into next IB, if this is ok for you, so as the rest can be done on a clean basis. Technically this PR can be merged on top of that (verified), you will just need to rerun code-checks.

@Dr15Jones
Copy link
Contributor Author

I am ready to integrate it into next IB, if this is ok for you, so as the rest can be done on a clean basis. Technically this PR can be merged on top of that (verified), you will just need to rerun code-checks.

@fabiocos I'm not sure which pull request you mean. If it is one of the code-format pull requests, then I'm all fine with first merging those.

@Dr15Jones
Copy link
Contributor Author

@fabiocos what about code-format for DataFormats/PatCandidates?

@fabiocos
Copy link
Contributor

@Dr15Jones yes, I mean merging #27278 tonight. For PatCandidate we do not have anything open yet as far as I can see: @smuzaffar is this in the pipe? In any case this PR has just these overlaps at present:

Testing PR #  27287  for branch  master
File  IOPool/Streamer/BuildFile.xml  modified in PRs #  ['27288', '27287']
File  IOPool/Streamer/src/StreamerInputSource.cc  modified in PRs #  ['27288', '27287']
File  PhysicsTools/CondLiteIO/plugins/FWLiteESRecordWriterAnalyzer.cc  modified in PRs #  ['26592', '27287', '27278']
File  PhysicsTools/CondLiteIO/src/RecordWriter.cc  modified in PRs #  ['27287', '27278']
File  Utilities/ReleaseScripts/scripts/git-publish  modified in PRs #  ['27287', '27230']

@smuzaffar
Copy link
Contributor

@Dr15Jones , go ahead and apply the code-format changes. I can redo the #27278 once this PR is merged.

@fabiocos
Copy link
Contributor

@Dr15Jones @smuzaffar #27278 has been merged without any conflict, I restart the code-checks

@fabiocos
Copy link
Contributor

code-checks

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@Dr15Jones
Copy link
Contributor Author

+1

@perrotta
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

@ggovi @efeyazgan @qliphy @santocch please check this PR

@fabiocos
Copy link
Contributor

@ggovi @efeyazgan @qliphy the additions to your areas look technical and straightforward, I would like to move forward this PR

@efeyazgan
Copy link
Contributor

+1

@santocch
Copy link

+1

fabiocos added a commit to fabiocos/cms-bot that referenced this pull request Jun 24, 2019
smuzaffar added a commit to cms-sw/cms-bot that referenced this pull request Jun 24, 2019
@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@fabiocos
Copy link
Contributor

@smuzaffar as expected this has broken again the FWLite build, the package list needs to be updated again with the new package added here

@smuzaffar
Copy link
Contributor

@fabiocos , this has been already fixed cms-sw/cmsdist#5056

@Dr15Jones Dr15Jones deleted the reflection branch June 25, 2019 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment