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

Delete an unused class rule #25670

Merged
merged 1 commit into from
Jan 17, 2019
Merged

Conversation

yamaguchi1024
Copy link
Contributor

ROOT::Math::MatRepSym<*> was defined, but was never used in selection
files.

ROOT::Math::MatRepSym<*> was defined, but was never used in selection
files.
@smuzaffar
Copy link
Contributor

ping bot

@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-25670/8013

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @yamaguchi1024 (Yuka Takahashi) for master.

It involves the following packages:

DataFormats/Math

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @rovere this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 15, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32620/console Started: 2019/01/15 17:34

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3153717
  • DQMHistoTests: Total failures: 13
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3153500
  • DQMHistoTests: Total skipped: 204
  • 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

@smuzaffar
Copy link
Contributor

ok thanks @yamaguchi1024 for the warning.

@slava77
Copy link
Contributor

slava77 commented Jan 16, 2019

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

I'm confused by this , because it's following

Comparison is ready

@smuzaffar
was the error message related to some test started manually by you?
Did that test fail due to the change here or due to some other problems?
Please clarify.
Thank you.

@cms-sw cms-sw deleted a comment from cmsbuild Jan 16, 2019
@smuzaffar
Copy link
Contributor

@slava77 , yes I wanted to test this PR againt CXXMODULE_X IBs where root has complaint about this unused dictionary but as CXXMODULE_X IB is fully broken that is why I killed the tests and removed the comments which were hiding the original tests results.

Thsi Pr should be good to go now. If still confused then we can restart the tests for normal 10.5.X :-)

@@ -2,9 +2,6 @@
<selection>
<class pattern="ROOT::Math::SMatrix<*>" />
<class pattern="ROOT::Math::MatRepStd<*>" />
<class pattern="ROOT::Math::MatRepSym<*>" >
<field name="fOff" transient="true"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for posterity: this data member was removed in 2015
root-project/root@9219906#diff-8e7303403181c06dc2d7e791c828076bL363

@slava77
Copy link
Contributor

slava77 commented Jan 16, 2019

+1

for #25670 81f77ec

  • code changes are in line with the PR description
  • jenkins tests pass and comparisons with the baseline show no relevant issues

@cmsbuild
Copy link
Contributor

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 now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

As the problems do not seem related with the regular IB, I will move forward with the integration of this PR for next IB

yamaguchi1024 added a commit to yamaguchi1024/cmsdist that referenced this pull request Jan 17, 2019
This PR should be tested with
cms-sw/cmssw#25670. This enables pcm to be fully
generated (Not yet fully loaded, though).
@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 6a7dbf7 into cms-sw:master Jan 17, 2019
yamaguchi1024 added a commit to yamaguchi1024/cmsdist that referenced this pull request Jan 18, 2019
This PR should be tested with
cms-sw/cmssw#25670. This enables pcm to be fully
generated (Not yet fully loaded, though).
yamaguchi1024 added a commit to yamaguchi1024/cmsdist that referenced this pull request Jan 21, 2019
This PR should be tested with
cms-sw/cmssw#25670. This enables pcm to be fully
generated (Not yet fully loaded, though).
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