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

shared_ptr has arrived to the standard. NFC #27625

Merged
merged 3 commits into from
Aug 8, 2019

Conversation

vgvassilev
Copy link
Contributor

@vgvassilev vgvassilev commented Jul 29, 2019

We replace the CPP11_shared_ptr macro with its actual expansion to
std::shared_ptr nowadays.

We replace the inclusion of <CPP11_shared_ptr.hh> with and order
the includes from less generic to generic ones where applicable.

Depends on: #27626

cc @davidlange6

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

We replace the CPP11_shared_ptr macro with its actual expansion to
std::shared_ptr nowadays.
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

We replace the CPP11_shared_ptr macro with its actual expansion to
std::shared_ptr nowadays.

We replace the inclusion of <CPP11_shared_ptr.hh> with <memory> and order
the includes from less generic to generic ones where applicable.
@vgvassilev vgvassilev force-pushed the align-shared-ptr-arrived branch from 67cf71f to 521a053 Compare July 29, 2019 02:52
@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-27625/11110

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

@vgvassilev
Copy link
Contributor Author

I do not think it makes sense to apply what the bot proposed.

@davidlange6
Copy link
Contributor

@fabiocos - is it possible that clang-format has not been run on Alignment/Geners/interface/ArchiveRecord.hh? (which does not follow the right naming convention)

@davidlange6
Copy link
Contributor

never mind - I realize its easy to answer my own question - clang-format has indeed not been run on this file for some reason (perhaps its name). @vgvassilev - the patch is basically orthogonal to your work, its just that you touched a file that eluded the clang-format migration

@smuzaffar
Copy link
Contributor

@davidlange6 , code-format was not run on this file as this file is not used by any thing. @vgvassilev I would suggest to just drop this file.

@vgvassilev
Copy link
Contributor Author

Done.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27625/11277

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2019

Pull request #27625 was updated. @christopheralanwest, @tocheng, @cmsbuild, @franzoni, @tlampen, @pohsun, @santocch can you please check and sign again.

@christopheralanwest
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1859/console Started: 2019/08/06 20:43

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2019

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0486aa/1859/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 2715989
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2715658
  • DQMHistoTests: Total skipped: 329
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@pohsun
Copy link

pohsun commented Aug 6, 2019

+1

@fabiocos
Copy link
Contributor

fabiocos commented Aug 8, 2019

+1

@santocch the PR looks ok to me, please have a look

@fabiocos
Copy link
Contributor

fabiocos commented Aug 8, 2019

merge

@cmsbuild cmsbuild merged commit 5ef351f into cms-sw:master Aug 8, 2019
@santocch
Copy link

santocch commented Aug 8, 2019

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 8, 2019

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

@vgvassilev vgvassilev deleted the align-shared-ptr-arrived branch August 9, 2019 06:33
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.

8 participants