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

Initial Python 2 removal #4091

Merged
merged 9 commits into from Oct 31, 2019
Merged

Initial Python 2 removal #4091

merged 9 commits into from Oct 31, 2019

Conversation

wRAR
Copy link
Contributor

@wRAR wRAR commented Oct 21, 2019

This pull request, split out from #3983, removes Python 2 support from documentation and basic stuff like CI.

This is #3994 but made as a local branch instead of a fork. It's also rebased to the current master.

@wRAR wRAR added this to the v2.0 milestone Oct 21, 2019
@wRAR wRAR mentioned this pull request Oct 21, 2019
@codecov
Copy link

codecov bot commented Oct 21, 2019

Codecov Report

Merging #4091 into master will decrease coverage by 2.4%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #4091      +/-   ##
==========================================
- Coverage   85.82%   83.42%   -2.41%     
==========================================
  Files         165      165              
  Lines        9851     9851              
  Branches     1464     1464              
==========================================
- Hits         8455     8218     -237     
- Misses       1139     1368     +229     
- Partials      257      265       +8
Impacted Files Coverage Δ
scrapy/__init__.py 85.71% <0%> (ø) ⬆️
scrapy/linkextractors/sgml.py 0% <0%> (-96.81%) ⬇️
scrapy/linkextractors/regex.py 0% <0%> (-95.66%) ⬇️
scrapy/linkextractors/htmlparser.py 0% <0%> (-92.07%) ⬇️
scrapy/extensions/statsmailer.py 0% <0%> (-29.17%) ⬇️
scrapy/_monkeypatches.py 54.54% <0%> (-18.19%) ⬇️
scrapy/link.py 86.36% <0%> (-13.64%) ⬇️
scrapy/utils/gz.py 92.1% <0%> (-7.9%) ⬇️
scrapy/utils/reqser.py 88.23% <0%> (-5.89%) ⬇️
scrapy/utils/python.py 78.72% <0%> (-5.86%) ⬇️
... and 13 more

.travis.yml Show resolved Hide resolved
@Gallaecio
Copy link
Member

Gallaecio commented Oct 30, 2019

This needs a deps line removal because of Python 3.8 support being added to master.

@Gallaecio Gallaecio merged commit 229e722 into master Oct 31, 2019
1 of 2 checks passed
* :ref:`topics-feed-storage-stdout`

Some storage backends may be unavailable if the required external libraries are
not available. For example, the S3 backend is only available if the botocore_
or boto_ library is installed (Scrapy supports boto_ only on Python 2).
Copy link
Member

@kmike kmike Oct 31, 2019

Choose a reason for hiding this comment

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

should we remove boto support from the code as well (sorry, probably this is already handled elswhere)?

Copy link
Contributor Author

@wRAR wRAR Oct 31, 2019

Choose a reason for hiding this comment

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

There is #1866, we need to decide on the solution

Copy link
Contributor Author

@wRAR wRAR Oct 31, 2019

Choose a reason for hiding this comment

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

(see my comment)

python: 2.7
- env: TOXENV=py27-extra-deps
python: 2.7
- env: TOXENV=pypy
Copy link
Member

@kmike kmike Oct 31, 2019

Choose a reason for hiding this comment

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

in install section there is code related to pypy

Copy link
Contributor Author

@wRAR wRAR Oct 31, 2019

Choose a reason for hiding this comment

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

@kmike indeed, what should we do about that section? Just remove the "$TOXENV" = "pypy" part?

Copy link
Member

@Gallaecio Gallaecio Oct 31, 2019

Choose a reason for hiding this comment

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

Remove the whole if … fi section of "$TOXENV" = "pypy", yes.

Copy link
Member

@Gallaecio Gallaecio Oct 31, 2019

Choose a reason for hiding this comment

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

You could also remove the pypy3 part, and use python: pypy3 in the matrix (see scrapy/w3lib#140)

Copy link
Contributor Author

@wRAR wRAR Oct 31, 2019

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants