fix: 🐛 double method definitions and python2 syntax#31
fix: 🐛 double method definitions and python2 syntax#31jonathan-schmieg merged 4 commits intooptile-headfrom
Conversation
|
|
||
| we want this version | ||
| """ | ||
| assert all(isinstance(element, str) for element in teams), teams |
There was a problem hiding this comment.
thought: Dear reviewer, you probably want to expand the diffs around the "we want this" so you can verify that the {replace,remove}_team_push_restrictions versions that we want to keep are indeed valid syntax
|
thought: I tried running a linter with reasonable defaults over this codebase, there are a huge number of errors. However, none are |
770aa93 to
d3e6460
Compare
ebcf2a3 to
ac9b96e
Compare
| 3.8: py38, docs, lint | ||
| 3.9: py39 | ||
| 3.8: py38 | ||
| 3.9: py39, docs, lint |
There was a problem hiding this comment.
thought: python 3.9 is what we use in ghconf-runner. That is only known current user of this code.
So we lint, docs, and typecheck on that
|
@jonathan-schmieg I think we should use your admin powers to force merge this PR. There is no chance that I'm getting CI to pass for this fork. GhConf is broken and I'm 99% sure that this PR will make it less broken if merged. I guess previous maintainers of this fork just force merged rather than running PRs with check suites. Especially since the last commit to the branch fixed completely invalid syntax that had been around for years |
GhConf run https://github.com/optile/ghconf-runner/actions/runs/16501328390/job/46660454633
fails with
NameError: name 'unicode' is not defined.Unsurprisingly, as that hasn't been valid python syntax in more than 5.5 years.
That invalid syntax was in 2 functions both of which were redefinitions of earlier, valid callables.
This PR removes the invalid syntax, we should now have the valid callables in the method resolution order.
Goodness knows how this ever worked. Clearly there is no test coverage here.
We must either have never hit the
{replace,remove}_team_push_restrictionsfunctions before, or only hit them is circumstances whereassertlines are swallowed (needs python compiler/interpreter to be compiled with unusual optimization flags)Relates to PCPAY-3844 and https://app.incident.io/payoneer-checkout/incidents/362