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

remove L1Trigger/L1TMuon dependency from DataFormats/L1TMuon #31417

Merged
merged 1 commit into from Sep 18, 2020

Conversation

davidlange6
Copy link
Contributor

new version of #31346 which stayed around long enough to get conflicts but no comments...

@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-31417/18285

@davidlange6
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 10, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlange6 (David Lange) for master.

It involves the following packages:

DataFormats/L1TMuon
EventFilter/L1TRawToDigi
L1Trigger/CSCCommonTrigger
L1Trigger/CSCTrackFinder
L1Trigger/CSCTriggerPrimitives
L1Trigger/L1TMuon
L1Trigger/L1TMuonEndCap
L1Trigger/L1TMuonOverlap
Validation/MuonCSCDigis

@benkrikler, @kmaeshima, @andrius-k, @cmsbuild, @rekovic, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @rovere, @valuev, @dildick, @thomreis, @ptcox this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

+1
Tested at: 0b34499
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-032b0e/9244/summary.html
CMSSW: CMSSW_11_2_X_2020-09-09-2300
SCRAM_ARCH: slc7_amd64_gcc820

@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-032b0e/9244/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2609667
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2609644
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
  • Checked 149 log files, 22 edm output root files, 35 DQM output files

@jfernan2
Copy link
Contributor

+1

#ifndef DataFormats_L1TMuon_L1TMuonSubsystems_hh
#define DataFormats_L1TMuon_L1TMuonSubsystems_hh

namespace L1TMuon {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a enum class be better since this is presently violating the CMS naming rules for namespaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this namespace is used extensively already...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but of course its easy to change if we don't mind it being different from the rest of the L1TMuon code.

@silviodonato
Copy link
Contributor

@cms-sw/l1-l2 do you have any comments?

@silviodonato
Copy link
Contributor

merge
@cms-sw/l1-l2

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