Clean BuildFiles after Reco in alphabet #29485
Conversation
|
The code-checks are being triggered in jenkins. |
|
Well these affects too many areas and collecting the signatures would take forever. Should be split up somehow. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29485/14700
|
|
A new Pull Request was created by @guitargeek (Jonas Rembser) for master. It involves the following packages: Alignment/OfflineValidation @andrius-k, @schneiml, @ianna, @kpedro88, @rekovic, @fioriNTU, @tlampen, @pohsun, @santocch, @perrotta, @civanch, @makortel, @cmsbuild, @smuzaffar, @Dr15Jones, @cvuosalo, @mdhildreth, @jfernan2, @tocheng, @slava77, @benkrikler, @kmaeshima, @christopheralanwest can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
Well, since this is already getting tested I might as well reopen it and give it a shot! |
|
-1 Tested at: 9cacd94 CMSSW: CMSSW_11_1_X_2020-04-15-1100 I found follow errors while testing this PR Failed tests: UnitTests
I found errors in the following unit tests: ---> test testPhase2PixelNtuple had ERRORS |
|
+1 |
|
+1 |
ok, then let me send a reminder to |
| <use name="TrackingTools/DetLayers"/> | ||
| <export> | ||
| <lib name="1"/> | ||
| </export> |
There was a problem hiding this comment.
Apparently this file was not needed before.
what does this do for a header-only package?
There was a problem hiding this comment.
Apparently this file was not needed before.
what does this do for a header-only package?
@smuzaffar Could you clarify?
There was a problem hiding this comment.
BuildFiles are useful for header only packages too. This way one exports the dependency so that the packages which use this do not have to explicitly add the missing dependencies. The <export> </export> section in this type of BuildFile(s) does not make any sence as there is no library generated for such packages. SCRAM silently ignores this. Better to remove the <export> </export> section here.
There was a problem hiding this comment.
Thanks, that's good too know! I was checking the wrong header-only package for inspiration: https://github.com/cms-sw/cmssw/tree/master/RecoCaloTools/Selector
@slava77, as this is in the reco area, how do you want to proceed? We can either change this now, or we rely for now on scram ignoring the export statement and remove it in another PR (which can also cover the other header-only libraries). I favour the latter solution, because then it doesn't reset all the signatures and we are almost there...
|
+1
|
|
+1 |
|
Hi @benkrikler @rekovic, hoping to make thinks faster here before the 100+ edited files collide with another PR :) The packages that fall under your area here are |
|
+1 |
cmsbuild
added
fully-signed
l1-approved
and removed
l1-pending
pending-signatures
labels
|
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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Another quick partially automatic BuildFile cleaning PR in the style of many before (for example #29295), this time covering all subsystems that come after Reco in the alphabet.
Nothing special this time, except for maybe moving a file
SimTracker/TrackerFilters/{interface → plugins}/CosmicTIFTrigFilter.hsuch that the header file and and the implementation are in the same (plugins) library and the dependencies are not mixed.PR validation:
CMSSW compiles. It was checked that all newly added dependencies are actually required by the package with
git grep.