Optionally skip creation of .gitignore in virtualenv directory by compwiztobe · Pull Request #2004 · pypa/virtualenv

compwiztobe · 2020-11-02T07:44:23Z

Addresses #2003. Creation of .gitignore with non-overridable * directive in the environment directory interferes with possibly more specific ignore directives in higher directories. When the environment directory is a subdirectory of a repo and we need tracking of a subset of its files, allow to skip creation of this .gitignore with a command line option to virtualenv.

Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)!

ran the linter to address style issues (tox -e fix_lint)
wrote descriptive pull request text
ensured there are test(s) validating the fix
added news fragment in docs/changelog folder
updated/extended the documentation

src/virtualenv/create/creator.py

codecov · 2020-11-02T07:47:37Z

Codecov Report

Merging #2004 into main will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2004      +/-   ##
==========================================
+ Coverage   94.27%   94.29%   +0.02%     
==========================================
  Files          86       86              
  Lines        4277     4280       +3     
==========================================
+ Hits         4032     4036       +4     
+ Misses        245      244       -1

Flag	Coverage Δ
tests	`94.29% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/virtualenv/create/creator.py	`96.15% <100.00%> (+0.09%)`	⬆️
src/virtualenv/seed/embed/base_embed.py	`98.11% <0.00%> (+1.88%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ae716f...6e13306. Read the comment docs.

if ever in the future it becomes possible to implement similar directives at the subdirectory level for Mercurial, Bazaar, SVN

docs/changelog/2003.feature.rst

src/virtualenv/create/creator.py

compwiztobe · 2020-11-02T08:35:51Z

src/virtualenv/create/creator.py

+            dest="vcs_ignore",
+            action="store_false",
+            help="don't create VCS ignore directive in the destination directory",
+            default=True,


Does this break convention and more importantly the CLI option-environment variable interop, by renaming the argument? I wasn't aware of the config-file/env-var pathway to set these, and now it's not clear to me if those will expect a value at no_vcs_ignore/VIRTUALENV_NO_VCS_IGNORE or vcs_ignore/VIRTUALENV_VCS_IGNORE or some combo.

So if you put:

[virtualenv] no_vcs_ignore = false

in your users virtualenv.ini file you enabled this flag always on your machine.

gaborbernat · 2020-11-02T09:30:59Z

src/virtualenv/create/creator.py

            default=False,
        )
+        parser.add_argument(
+            "--no-vcs-ignore",


VIRTUALENV_NO_VCS_IGNORE and no_vcs_ignore will be the names as documented under https://virtualenv.pypa.io/en/latest/cli_interface.html#environment-variables. The destination variable doesn't matter only the CLI flag.

gaborbernat · 2020-11-02T09:33:07Z

src/virtualenv/create/creator.py

+            dest="vcs_ignore",
+            action="store_false",
+            help="don't create VCS ignore directive in the destination directory",
+            default=True,


So if you put:

[virtualenv] no_vcs_ignore = false

in your users virtualenv.ini file you enabled this flag always on your machine.

gaborbernat · 2020-11-02T09:34:31Z

src/virtualenv/create/creator.py

        )
+        parser.add_argument(
+            "--no-vcs-ignore",
+            dest="vcs_ignore",


I don't like how you negated the flag name, but kept the destination positive. Please keep them in sync. This flag should be --no-vcs-ignore, destination no_vcs_ignore and should store_true with default False.

Done. Wasn't really any reason for old way, other than the double negative on line 172, and "because I can". But bringing into sync also will make the default value in auto-docs less confusing (actually correct).

pradyunsg · 2020-11-02T11:06:37Z

I usually find is really awkward to have double-negatives for configuration of tooling (pip is HORRIBLE on this front).

Here's my 2 cents: if we're doing this, let's add --vcs-ignore/--no-vcs-ignore (I don't care about the exact naming) such that the configuration options look like vcs_ignore = false. By having on/off knobs for the boolean, we'd also allow folks to override environment-level "no" with a "yes" CLI flag.

gaborbernat · 2020-11-02T11:31:16Z

Here's my 2 cents: if we're doing this, let's add --vcs-ignore/--no-vcs-ignore (I don't care about the exact naming) such that the configuration options look like vcs_ignore = false. By having on/off knobs for the boolean, we'd also allow folks to override environment-level "no" with a "yes" CLI flag.

Supporting --flag/--no-flag would be a generic feature we don't do at the moment and I'm personally not totally sold on it (or interested in implementing it). Either way, would this be done I would not single out a new flag, but implement it for all boolean flags, which is clearly out of the scope of this PR.

gaborbernat

Optionally skip creation of .gitignore in virtualenv directory

43d27d7

gaborbernat requested changes Nov 2, 2020

View reviewed changes

src/virtualenv/create/creator.py Show resolved Hide resolved

update git -> VCS for implementation agnosticism

8025d4e

if ever in the future it becomes possible to implement similar directives at the subdirectory level for Mercurial, Bazaar, SVN

gaborbernat requested changes Nov 2, 2020

View reviewed changes

docs/changelog/2003.feature.rst Outdated Show resolved Hide resolved

docs/changelog/2003.feature.rst Outdated Show resolved Hide resolved

src/virtualenv/create/creator.py Outdated Show resolved Hide resolved

src/virtualenv/create/creator.py Outdated Show resolved Hide resolved

compwiztobe force-pushed the no-gitignore branch from bbb2dae to 2af40cb Compare November 2, 2020 08:28

address gaborbernat comments

1a28968

compwiztobe force-pushed the no-gitignore branch from 2af40cb to 1a28968 Compare November 2, 2020 08:30

compwiztobe commented Nov 2, 2020

View reviewed changes

gaborbernat reviewed Nov 2, 2020

View reviewed changes

gaborbernat requested changes Nov 2, 2020

View reviewed changes

match bool polarity to avoid confusion

6e13306

gaborbernat approved these changes Nov 2, 2020

View reviewed changes

gaborbernat merged commit 9708b54 into pypa:main Nov 2, 2020

compwiztobe deleted the no-gitignore branch November 2, 2020 11:49

Uh oh!

Conversation

compwiztobe commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)!

Uh oh!

Uh oh!

codecov bot commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compwiztobe Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

gaborbernat Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

compwiztobe Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

gaborbernat Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

gaborbernat Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

gaborbernat Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

compwiztobe Nov 2, 2020

Choose a reason for hiding this comment

Uh oh!

pradyunsg commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaborbernat commented Nov 2, 2020

Uh oh!

gaborbernat left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

compwiztobe commented Nov 2, 2020 •

edited

Loading

codecov bot commented Nov 2, 2020 •

edited

Loading

pradyunsg commented Nov 2, 2020 •

edited

Loading