[build-script] Move some calculations into independent methods. #23865
Conversation
Move the calculations of platforms_to_skip_build, platforms_to_skip_test, platforms_archs_to_skip_test, and platforms_to_skip_test_host to their own independent functions. Each function deal with one of them and they are pure functions, which should improve readability a little.
|
@swift-ci please test |
| self.platforms_to_skip_build = set() | ||
| self.build_libparser_only = args.build_libparser_only | ||
|
|
||
| def __platforms_to_skip_build(self, args): |
There was a problem hiding this comment.
Nitpick: I think the Python standard for "private" methods is to prefix with a single _ character. Certainly not a big issue.
There was a problem hiding this comment.
One underscore is a “agreement” of a non-public API, but completely accessible. At least two underscores (and not finishing in more than one underscore) will ask the Python interpreter to mangle the name, becoming a little bit more “private” (but the mangling scheme is quite simple, so still accessible).
(Ref: https://docs.python.org/2.7/tutorial/classes.html#private-variables-and-class-local-references)
In this case in particular, it doesn’t matter that much which one to use (anything that indicates “don’t call this” should be fine), but in other cases (specially when inheritance is involved), the two underscores avoids name clashing problems.
There was a problem hiding this comment.
I think I remember seeing something along these lines a long time ago, but I totally forgot. That's a pretty cool feature! I'm totally fine with keeping these names then considering these are implementation details.
Move the calculations of platforms_to_skip_build, platforms_to_skip_test, platforms_archs_to_skip_test, and platforms_to_skip_test_host to their own independent functions. Each function deal with one of them and they are pure functions, which should improve readability a little.
This commit is part of #23810 which was reverted in #23861. This commit only deals with a split of a large method into smaller function in the context of
BuildScriptInvocation.This was part of #23257. I’m trying to split the PR in smaller ones to make the reviews easier. Other PRs in this group are #23303, #23803, #23810, and #23822.