pytest-dev / pytest-forked

extracted --boxed from pytest-xdist to ensure backward compat
MIT License
62 stars 21 forks source link

Exclude .gitignore and .travis.yml from distribution #43

Closed swt2c closed 4 years ago

webknjaz commented 4 years ago

Why? I'd rather remove the manifest altogether because the files are tracked by the setuptools-scm integration.

swt2c commented 4 years ago

Because MANIFEST.in is still the way to exclude files, even when using setuptools_scm. See: https://pypi.org/project/setuptools-scm/

webknjaz commented 4 years ago

Right, but what's the motivation here? These files are not that big, having them is not harmful.

swt2c commented 4 years ago

Right, but what's the motivation here? These files are not that big, having them is not harmful.

They are harmful, for example, if the source package ends up getting imported to another VCS, e.g., Debian packaging.

webknjaz commented 4 years ago

I guess this is subjective. I don't really see any problem there. Besides, OS package managers usually apply patches anyway and we shouldn't be held responsible for how they solve this.

swt2c commented 4 years ago

I guess this is subjective. I don't really see any problem there. Besides, OS package managers usually apply patches anyway and we shouldn't be held responsible for how they solve this.

So, you want every Linux distribution to have to remove this stuff because you don't want to maintain a 2-line file?

webknjaz commented 4 years ago

Essentially, yes. In fact, this library is not being actively developed.

swt2c commented 4 years ago

Why are you even reviewing my PR then? This is very frustrating for a trivial change.

webknjaz commented 4 years ago

I'm reviewing because nobody else is going to. And also I had a minute + got commit bit recently. So technically I can merge changes that seem reasonable. Though, I'm not going to invest much time here anyway.

I just wanted to state my position that I only want to care about the dist metadata in the context of publishing to PyPI, hence the Python ecosystem. If each Python project was to support each and every OS packaging case, it'd be a mess and a lot of maintenance burden so this is the policy I apply in the projects I maintain.

If somebody else is going to be responsible for such things, it's different from when I'm being told what to maintain and I opt-out of it.

It doesn't mean that somebody else won't merge this but it's unlikely: Ronny mentioned that he is not going to be maintaining this lib.

Anyway, I've just wanted to say that it's not important/breaking enough for us to care. And fewer things to maintain is better. An example of when it'd be important would be a case with some large unnecessary blob included. But special-casing every each config seems not right.

swt2c commented 4 years ago

Sure, I don't argue that this is a high priority issue. It's just a low priority but trivial thing that should be no maintenance burden and seems to be the right thing to do. Would it make you happy if I deleted the rest of the lines in the file for a net deletion of lines? :-)

BTW, Ronny has commented before that SCM/CI data doesn't belong in sdist: https://github.com/pytest-dev/pytest-xdist/issues/161#issuecomment-308406325

webknjaz commented 4 years ago

I mean, he said "excludes are sometimes nice" and that such things "don't belong in sdist". And I agree with that, I just wanted to say that even if they are there, they are harmless. So the point of trying to maintain such exclude-lists in an unmaintained project doesn't seem important enough. You exclude one thing, and then another. And before you know it, you're surrounded by all of these small things you commit to maintaining. I think that for unmaintained projects, only really dangerous things (like security bugs or stuff that damages env integrity) should be important.

webknjaz commented 4 years ago

FWIW in the very same thread that you linked, Bruno expressed the same sentiment as I do here. And it has lead to removing MANIFEST.in instead of trying to keep it up-to-date.

swt2c commented 4 years ago

Actually, perhaps an even better idea would be for setuptools_scm to exclude these types of files automatically. Perhaps I'll propose that.

webknjaz commented 4 years ago

Closing per above.