Fix for tox4 regression issue with setenv file and substitutions (#2435)#3521
Fix for tox4 regression issue with setenv file and substitutions (#2435)#3521gaborbernat merged 5 commits intotox-dev:mainfrom
Conversation
src/tox/config/set_env.py
Outdated
| if not self._is_file_line(sub_line): | ||
| sub_raw.__setitem__(*self._extract_key_value(sub_line)) | ||
| else: | ||
| for k, v in self._stream_env_file(self._parse_file_line(sub_line), args): |
There was a problem hiding this comment.
No single letter variables outside of comprehensions.
| line = self._needs_replacement.pop(0) | ||
| expanded_line = self._replacer(line, ConfigLoadArgs([], self._name, self._env_name)) | ||
| sub_raw = dict(self._extract_key_value(sub_line) for sub_line in expanded_line.splitlines() if sub_line) | ||
| expanded_line = self._replacer(line, args) |
There was a problem hiding this comment.
Can you explain the change here and why you done it this way?
There was a problem hiding this comment.
- [+] wrote descriptive pull request text
This part you didn't really address. Writing a descriptive text involves the approach you've taken and why.
There was a problem hiding this comment.
Yeah, that's ConfigLoadArgs variable is being used twice now since the call to _stream_env_file(_read_env_file) requires it. Seems like no point in instantiating it twice.
There was a problem hiding this comment.
Well, the description is pointing out to the long standing regression issue that wasn't fixed yet, this is an attempt to do it, since it was stated there "PR welcome to fix it.", what else could be said. 🤷
There was a problem hiding this comment.
Yeah, that's
ConfigLoadArgsvariable is being used twice now since the call to _stream_env_file(_read_env_file) requires it
Was looking for an explanation for this, as in why we need to call it twice now?
realitycheck
force-pushed
the
main
branch
2 times, most recently
from
2ed59d3 to
9cc6076
Compare
gaborbernat
left a comment
gaborbernat
left a comment
There was a problem hiding this comment.
Let's add changelog entry 😊
2 participants
This PR contains fix for the issue #2435. (#2435 (comment))
Please, make sure you address all the checklists (for details on how see
development documentation)! -->
tox -e fix)docs/changelogfolder