Re: [Twisted-Python] Developer docs should be updated on wiki when steps changed in code?

[Glyph]

> On Feb 3, 2021, at 4:51 AM, Craig Rodrigues  wrote:
> 
> In this PR: https://github.com/twisted/twisted/pull/1461 
> 
> some changes were made to the Twisted development process
> for running the various linters before committing code.
> 
> I noticed that the changes in PR 1461 were not reflected
> in docs on the wiki, specifically here: 
> https://twistedmatrix.com/trac/wiki/TwistedDevelopment 
> 
> 
> which mentions tox environments that have been deleted in PR 1461.
> 
> I noticed this, when I recently tried to work on a branch from recent trunk,
> and when I tried to run some of the commands on that wiki page
> they didn't work, because they were deleted.
> 
> In this post-review comment:
> https://github.com/twisted/twisted/pull/1461#issuecomment-753695414 
> 
> 
> I asked Thomas if he could modify the wiki to reflect the changes he made in 
> trunk,
> but this hasn't been done yet.
> 
> My impression in the comments in PR 1461 is that Thomas and Adi want
> to move all this documentation into the source tree, and delete the stuff on 
> the wiki?
> 
> Is this the long term plan?

I can't quickly find the place where we agreed to this, but I think several 
years ago at this point we had a discussion about moving all these docs into 
the source tree.  (If they're on the wiki, there's no review process or even a 
place where updates can be staged for commentary before going live and becoming 
"official".)

> Until this is fully completed, what is the correct thing to do when there is 
> a mismatch between docs on the wiki, and docs in the tree?

Docs in the tree always win.

> I still refer to a lot of docs on the wiki, especially for process related 
> things,
> so I think it would be nice if the wiki docs were kept up to date, until the 
> day that
> they are fully deleted.

Let's start deleting them now, and replacing them with links to the in-tree 
docs, rather than updating them.  They've been skewing out of date for a long 
time.  When I was looking for information about how to do a revert, I found 
wiki docs about linking to revisions in Subversion which didn't mention Github, 
which gives a flavor for how outdated some of this stuff is.

Thomas, would you mind doing the honors for this document?  Links for process 
docs should probably be to https://docs.twistedmatrix.com/en/latest/ 
 since, for process information 
(unlike API information), trunk should be authoritative, not the latest release.

-g

> --
> Craig
> ___
> Twisted-Python mailing list
> Twisted-Python@twistedmatrix.com
> https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

[Twisted-Python] Developer docs should be updated on wiki when steps changed in code?

[Craig Rodrigues]

In this PR: https://github.com/twisted/twisted/pull/1461
some changes were made to the Twisted development process
for running the various linters before committing code.

I noticed that the changes in PR 1461 were not reflected
in docs on the wiki, specifically here:
https://twistedmatrix.com/trac/wiki/TwistedDevelopment

which mentions tox environments that have been deleted in PR 1461.

I noticed this, when I recently tried to work on a branch from recent trunk,
and when I tried to run some of the commands on that wiki page
they didn't work, because they were deleted.

In this post-review comment:
https://github.com/twisted/twisted/pull/1461#issuecomment-753695414

I asked Thomas if he could modify the wiki to reflect the changes he made
in trunk,
but this hasn't been done yet.

My impression in the comments in PR 1461 is that Thomas and Adi want
to move all this documentation into the source tree, and delete the stuff
on the wiki?

Is this the long term plan?

Until this is fully completed, what is the correct thing to do when there
is a mismatch between
docs on the wiki, and docs in the tree?

I still refer to a lot of docs on the wiki, especially for process related
things,
so I think it would be nice if the wiki docs were kept up to date, until
the day that
they are fully deleted.

--
Craig
___
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Re: [Twisted-Python] resolving release management conflict

[Craig Rodrigues]

On Mon, Feb 1, 2021 at 1:00 PM Glyph  wrote:

>
> I believe the proximate cause of the conflict here is that we're dealing
> with this regression incorrectly.  When a regression is introduced, there's
> a process for dealing with it, originally documented here:
> https://twistedmatrix.com/trac/wiki/ReviewProcess#Revertingachange. [1]
>  The principle we need to outline clearly is this: if we have a regression,
> the change that introduced it should be reverted as soon as possible.  If
> we *can't* revert it directly because of changes to trunk in the
> meanwhile that introduce conflicts, ideally the change that we would make
> would be the closest possible thing to reverting it, even if that
> reintroduces a known bug that we have to reopen.
>
> In the case where a lot of work has happened in the interim, and it's *less
> work* to just "fix forward" on top of trunk, it's fine to do a small fix
> as an end run around this process.  The goal of this process is to minimize
> the work required to restore trunk to a releasable state.
>
> Right now we have three competing fixes which have all been flagged as
> incorrect in various ways:
>
>
>- from Tom: https://github.com/twisted/twisted/pull/1499
>   - Craig rejected this one by saying it will break "certain
>   versions" of Python, but I don't understand why it was closed rather 
> than
>   reviewed / reassigned as normal.  (Or for that matter which versions are
>   the problems, or how it breaks them.)
>
>
Yes, that is correct.  Over the summer, while investigating failures on
Azure CI caused by to non-ASCII characters in
a submitter's GitHub user ID.  I spent a lot of time testing in a Windows
environment on Python 3.5, 3.6, 3.7, and 3.8.
I found that there were subtle differences in the default encoding on the
console between each Python version,
and that affected the logic in _checkProcessArgs().
The fix that I did over the summer addressed the problems and fixed all the
CI issues.

There were two corner cases that were uncovered.  One by me, and one by Tom
Most.

CORNER CASE 1:In Buildbot, they patch sys.stdout with io.StringIO in
their tests which affects their tests which call the Twisted Reactor.
   io.StringIO is not a 100% proper
replacement for sys.stdout which is of type _io.TextIOWrapper on Python 3+.
   However, Buildbot's tests have been
around for a long time, so I decided

CORNER CASE 2:In a Windows GUI Application, sys.stdout is None.  So you
cannot get the encoding from sys.stdout.encoding


In https://github.com/twisted/twisted/pull/1502, I have a very small patch
which addresses those two corner cases.
I need to write a good test for it, but I think it is good enough to go
into the release.

Post-release:
(1) I would like to drop Python 3.5
(2)  Look at removing the _checkProcessArgs() function.  The complexity of
bytes/unicode logic in there was more useful
   in the Python 2 days when dealing with the differences between how
Python on Windows handled args/env vs.
   how Python on POSIX handled args/env.  I think this function can
completely go in a Python 3.6+ world, and will greatly simplify the code.
   However, this needs to be carefully tested and reviewed.



>
>- from Adi: https://github.com/twisted/twisted/pull/1501
>   - Craig rejected this one by saying "please stop working on this"
>   but it also doesn't have any technical reason that this solution is
>   incorrect.
>
> Yes, that is correct.  Sorry if that was an aggressive action on my part,
but I did a fix for this, as described above.






>
>- from Craig: https://github.com/twisted/twisted/pull/1502
>   - This was originally given a passing review by Povilas from the
>   Buildbot project, but twm added an additional blocking review shortly
>   thereafter explaining that it introduced an additional regression.  It
>   seems as though this one is now actively being worked on?
>
>
> Craig: you closed two of these issues on the basis that you were "already
> working on a fix", but that's not a valid reason to *close* a PR.  Or,
> for that matter, to give it a failing review.  In the future, please
> comment on PRs that you don't want merged by pointing out their technical
> insufficiency, so that we can take the opportunity to learn for the benefit
> of future maintenance.  In this case, the underlying issue is clearly quite
> nuanced and requires expertise that touches platform differences in
> Python's core APIs, so having multiple people working on different fixes is
> totally fine; this effort is not wasted if everybody learns a little bit
> about the code in question, and it's *definitely* not wasted if we have
> to synthesize our approaches in the end anyway because they address
> different cases.
>

OK, thanks for the feedback, and sorry for any process violations or hurt
feelings from my actions.



>
> All of these fixes have