Conversation
371cb60 to
eabf094
Compare
eabf094 to
330b4db
Compare
All but one of the tests in this test module are actually testing env_select functionality through calling other code that in turn uses env_select (CliEnv, EnvSelector and register_env_select_flags). We move that one test to the top of the file, but we do not indicate the boundary between the groups (or even explain that the grouping exists) at the request of @gaborbernat.
Some of the behaviour here, such as treatment of zero-length environment names specified by the user, is not really intentional, but apparently an artifact of the method used to parse the environment name list supplied by the user.
I find this makes the tests significantly less readable, so I'm commiting this separately so we can decide what to do about it. The options are: 1. Squash this with the previous, just using the auto-enforced formatting. 2. Leave these commits as-is, so we at least can go back to the more readable original formatting. 3. Do whatever needs to be done to keep the auto-reformatter from touching that particular bit of code. I have a preference for 3, but won't argue any other decision.
330b4db to
c53fefb
Compare
|
I'm not clear on why this PR is still saying that there are changes requested. I clicked "See review" and it just takes me to requests that are from before I pushed up the fixes and that are marked outdated and resolved. Can someone explain to me what's happening here? |
Until I approve, will stay in that status. That's how GitHub works. |
Ah, ok! I've seen something else similar; GitHub keeps saying things that make me wonder if I missed fixing something. But I'll just stop worrying about it. Please do poke me if you see a PR where I've responded to things but there are still things that I need to fix, thoug. |
This is a start on issue #3199. The plan for that involves changing CliEnv; this starts documenting its current behaviour by adding tests for it.
The final reformatting commit may need to be squashed into the previous commit; see that commit message for details.
No news fragment because this isn't a user-visible change. No documentation yet because that will be informed by review of this commit.
tox -e fix)docs/changelogfolder