Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop support for EOL Python <2.7 and 3.2-3.3 #322

Merged
merged 8 commits into from
Mar 25, 2018
Merged

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Jan 5, 2018

Fixes #319.

This builds on #303, drops EOL versions and does some cleanup. It also adds python_requires to setup.py to help pip.

Copy link
Member

@bitglue bitglue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on board. I retried the Travis build and it passed; any idea what's up with appveyor?

.gitignore Outdated
@@ -1,6 +1,7 @@
*.egg
*.egg-info
*.pyc
.idea
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add these to a global gitignore rather than the project

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this belongs in the project gitignore and cannot go in the global gitignore because some projects require some of the files from this directory to be in Git.

See https://intellij-support.jetbrains.com/hc/en-us/articles/206544839 for more info.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh? That you've put it in .gitignore explicitly states those files should not be tracked in git. IntelliJ is a detail of your preferred development environment, and not Pyflakes. That's why it should go in your personal .gitignore, not Pyflakes.

I use Vim which puts a .swp file next to whatever I'm editing. Some people use editors which make backups with a ~ suffix. But those aren't in here either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, I'm saying none of those files from the .idea dir should be in this project's repo.

But some other projects do want to share IDE config via some files from the .idea dir. So it's not a global level thing, it's a project-level thing.

I assume Pyflakes does not want any of that config, hence ignoring the entire dir in this case.

What sort of problems does it cause to have an extra entry in this gitignore?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hugovk when people insist upon thrusting their personal editor's junk into project gitignore files, it invites lots of unwelcome and unhelpful pull requests to add ever more editor junk files to the gitignore, usually by people who otherwise don't contribute to a project. I'm in agreement with @bitglue that this doesn't belong in this project's .gitignore. If projects want to track that information, then it can be added inspite of your global gitignore. What your global gitignore prevents, however, is adding it blindly with git add ..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also follow the instructions for a global gitignore above, but omit the --global option. This will change the setting just for the repo, so (just for pyflakes) you can use an alternative .gitignore. I'm not sure if that supersedes or arguments the default .gitignore, but you may want to experiment.

You might also be able to put a fully qualified path in your global .gitignore to ignore .idea in pyflakes, but not your other git repositories. Again I'm not sure this works, but something to try.

Prepending a leading ! to a line in gitignore can be used to unignore a file. I'm not familiar with the exact semantics of this feature as I've not had reason to do use it, but perhaps you could add it to projects for which sharing IDE files is explicitly desired.

In any case, the .gitignore here does not currently contain any entries that reflect developer preference: only those due to pyflakes and its associated tooling in this repository. That's unlikely to change.

Regardless, thank you for the contributions. They are very much appreciated. I should be able to get this merged and released soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, thanks for the tips on unignoring the global ignore, I'll give that a shot! I've removed it from this project's one!

@bitglue
Copy link
Member

bitglue commented Jan 9, 2018

Also, we should probably merge this and #303 together and release a new major version since it breaks backwards compatibility.

@hugovk
Copy link
Contributor Author

hugovk commented Jan 9, 2018

Appveyor looks like it needs an updated pip to recognise the new python_requires. I had a go updating it in another branch, but there's a pypy bug preventing that. They've just fixed it, and it'll be in the next pypy release, 5.10.1.

See https://bitbucket.org/pypy/pypy/issues/2720/ensurepip-on-pypy-c-jit-93579-a4194a67868f and https://stackoverflow.com/q/47999518/724176.

I do not know when that will be out. Options are to wait until it's sorted until merge, or comment out that pypy build until then. (Merge with the failure doesn't really help.)

@bitglue
Copy link
Member

bitglue commented Jan 9, 2018

Let's just comment that build with an explanation for now.

@hugovk
Copy link
Contributor Author

hugovk commented Jan 10, 2018

Commented out!

myint added a commit to PyCQA/autoflake that referenced this pull request Jan 12, 2018
This corresponds with the upcoming change in `pyflakes`:

PyCQA/pyflakes#322
@myint myint merged commit a13dbe6 into PyCQA:master Mar 25, 2018
@hugovk hugovk deleted the rm-eol branch March 25, 2018 20:16
@hugovk hugovk mentioned this pull request Mar 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants