-
Notifications
You must be signed in to change notification settings - Fork 108
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
Allow list of multiple architectures #11228
Conversation
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
CMSSW and Python look healthy:
I have squashed the commits and I think this is ready for review |
Jenkins results:
|
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.
Thanks Kenyi, it's looking good for me. I have left a few comments along the code for your consideration as well. In addition to those, can you please check the pycodestyle report for test/python/WMCore_t/BossAir_t/BasePlugin_t.py (there are a few new items reported here).
self.assertEqual(bp.scramArchtoRequiredArch( | ||
['slc7_amd64_gcc10', 'slc7_aarch64_gcc700', 'slc7_ppc64le_gcc9']), 'X86_64') | ||
self.assertEqual(bp.scramArchtoRequiredArch(['slc7_aarch64_gcc700', 'slc7_ppc64le_gcc9']), 'ppc64le') | ||
test = (bp.scramArchtoRequiredArch(['slc7_amd64_gcc10', 'slc7_aarch64_gcc700'])).split(',') |
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.
I think one pair of parenthesis here is not really needed (the first one).
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.
I think I got an error with .spllit()
without the parenthesis
Jenkins results:
|
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.
It looks good to me, thanks Kenyi.
@belforte FYI, in case you are using one of these APIs mapping ScramArch to OS/Arch.
Fixes #10674
Status
In development
Description
Is it backward compatible (if not, which system it affects?)
YES