osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Python-Dev] I reverted "Add Windows App Store package" change


Le ven. 7 d?c. 2018 ? 16:16, Steve Dower <steve.dower at python.org> a ?crit :
> The other changes are either in Windows-only files or tests. The one
> exception is venv, where they are in "if os.name=='nt'" blocks. I also
> pinged our venv expert a few times with no response.

Yeah, the lack of review is an issue in Python, I'm well aware of that...

But I guess that it would be easier to get a review on a change
modifying a single file (venv) than one PR modifying 52 files?
Usually, I'm scared by giant PRs and I just skip them :-)


> The PR was put up two *months* ago, not one week.

Oh, wait... I read Oct 30 and I counted 7 days... sorry :-) November
becomes December, not October. Sorry about that.


> That said, it's totally my fault for merging and then not watching.

That's fine. A revert doesn't mean that your change is wrong. The only
intent is to repair the CI as soon as possible to spot other
regressions, and give us more time to design the proper fix:

   https://pythondev.readthedocs.io/ci.html#revert-on-fail


> (Can we still do that? I'm not seeing any UI right now... I did run a number of test
> release builds on the release machine, so I knew that was going to be okay.)

The current process is described at:
https://devguide.python.org/buildbots/#custom-builders


> Historically, when changes to the part you point out have been extracted
> out from their context, they've been rejected on the basis that they
> aren't necessary.

I don't think that there is a general rule. It's usually decided on a
case by case basis, and I know that each core dev has a different
opinion on this question :-) I do prefer multiple small commits.

I would be simpler if it would be possible to have a "patch serie":
list of pull requests, or a single PR with multiple commits but don't
squash them.


> As it happens, I split out many changes to do with pathconfig that came
> from this. You rejected them because they weren't necessary :)

I'm sorry, I don't recall, which changes?


> I hope that explains a bit better.

It does, thanks :-)

Victor