On 2018-10-02, Michael Felt wrote:
> I am sorry, for myself obviously - but also for Python. Obviously, I am
> doing it all wrong - as I see lots of other issues being picked up
> immediately.
I'm not sure that's the case. There are a lot of PRs or bugs that
sit there without getting reviews. The problem is that few (or no)
core developers get paid to work on Python. So, the time they spend
is motivated by their specific "itch". Getting reviews on any PR is
difficult, even for core developers. In their case, they have to
option of forcing the issue, I guess.
This is a problem we should try to deal with somehow. Turning off
valuable contributors like you is bad. I'm not sure how to do it
though. At the core Python sprint in September there was some talk
about how CPython developers might get funding. Maybe that could
help deal with the backlog of reviews required.
> And, while you may not give a damn about anything other than Windows,
> macos and/or Linux - there are other platforms that would like a stable
> Python.
There is probably some truth in not caring about other platforms.
The problem from the reviewer perspective is the question of "what
is the potential downsides of this PR vs what are the benefits?".
The safest thing is to not approve the PR. No core developer wants
to be the person who broke CPython. You must admit, AIX is an
extremely niche platform at this point. I bet if you picked 1000
software developers at random, it would be likely that zero of them
have ever used AIX. So, it's not that we don't care at all about
AIX but that the cost/benefit equation makes accepting AIX specific
changes more difficult.
One specific suggestion I have about your PR is to try to make your
changes not AIX specific. Or at least, make the AIX checking as
localized as possible. So, as an example, in test_uuid you have:
_notAIX = not sys.platform.startswith("aix")
then later in the module you check that flag. While that is the
most direct approach to fixing the issue and making the test pass,
it is not good for the long term maintainability of the code. You
end up with boolean flags like _notAIX spread about the logic. Over
time, code like that becomes a nightmare to maintain.
Instead, I would suggest test_uuid is making platform specific
assumptions that are not true on AIX and possibly other platforms.
So, do something like:
_IS_AIX = sys.platform.startswith("aix")
_HAVE_MACADDR = (os.name == 'posix' and not _IS_AIX)
@unittest.skipUnless(_HAVE_MACADDR, 'requires Posix with macaddr')
def test_arp_getnode(self):
...
The _HAVE_MACADDR test is relatively simple and clear, does this
platform have this capability. Later in the code, a check for
_HAVE_MACADDR is also quite clear. If someone comes along with
another platform that doesn't support macaddr, they only have to
change one line of code.
This kind of capability checking is similar to what happened with
web browsers. In that case, people discovered that checking the
User Agent header was a bad idea. Instead, you should probe for
specific functionality and not assume based on browser IDs. For the
macaddr case, is there some way to you probe the arp command to see
if supports macaddr? That way your test doesn't have to include any
AIX specific check at all. Further, it would have some hope of
working on platforms other than AIX that also don't support macaddr
but are POSIX and have 'arp'. The code could be something like:
_HAVE_MACADDR = False
if os.name == 'posix':
if <check arp to see if it supports macaddr>:
_HAVE_MACADDR = True
Hope that is helpful.
Neil
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
