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
castor dqm update #24142
castor dqm update #24142
Conversation
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24142/5829 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24142/5829/git-diff.patch You can run |
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24142/5830 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24142/5830/git-diff.patch You can run |
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24142/5831 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24142/5831/git-diff.patch You can run |
Hi, Are you sure that code checks in this case didn't pass because of the bot error? Could you please review the code one more time? |
Hi,
checked again in version 10_3_0_pre1 (run320317 RAW): works without problem
scram build code-checks
gives Warnings:
/afs/cern.ch/user/p/popov/scratch_bk/workDQMthread3/CMSSW_10_3_0_pre1/src/DQM/CastorMoni
tor/interface/CastorMonitorModule.h:82:8: warning: annotate this function with 'override
' or (rarely) 'final' [modernize-use-override]
void analyze(const edm::Event& iEvent, const edm::EventSetup& eventSetup);
^ ~
override
which annotation must be: 'override' or 'final' ?
|
Hi, Yes the code seems to be fine functionally, but code checks still fail because of the styling issues. In particular it seems that you removed override statements from the methods which are actually overriding base class methods. Please see this bot comment. Override statements usually have no impact on functionality but they are good tool to avoid errors in the future, and they are required by CMSSW policy. |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24142/6176 |
Pull request #24142 was updated. @kmaeshima, @cmsbuild, @andrius-k, @jfernan2, @schneiml can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
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) |
Thank you for applying the the styling rules. Now code looks much nicer! |
+1 |
int ChannelStatus[14][16]; | ||
int N_GoodChannels = 224; | ||
int EtowerLastModule = 5; | ||
int TrigIndexMax = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why were the non-const global statics accepted as OK for production code?
@fabiocos please take a note
@slava77 thanks for catching the problem |
rm CastorBaseMonitor module
removed some TH2D histograms :
instead of 2xTH2D add TProfile
add Energy (digi) vs BX (latency)
re-implement DB conditions for masking