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

Make BaseItemExporter’s dont_fail parameter keyword-only #4370

Merged
merged 1 commit into from Feb 25, 2020

Conversation

Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Feb 24, 2020

Make the new dont_fail parameter of BaseItemExporter keyword only.

Otherwise, we are adding an non-intuitive positional parameter to a class that used to require keyword parameters only.

dont_fail was introduced after 1.8, so it would be great to have this change in for 2.0, otherwise we will need to introduce this change with a deprecation message for positional argument usages.

Split out from #4329

@codecov
Copy link

@codecov codecov bot commented Feb 24, 2020

Codecov Report

Merging #4370 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4370      +/-   ##
==========================================
- Coverage   84.14%   83.99%   -0.16%     
==========================================
  Files         166      166              
  Lines        9965     9965              
  Branches     1481     1481              
==========================================
- Hits         8385     8370      -15     
- Misses       1323     1340      +17     
+ Partials      257      255       -2     
Impacted Files Coverage Δ
scrapy/robotstxt.py 75.30% <0.00%> (-22.23%) ⬇️
scrapy/core/downloader/__init__.py 90.90% <0.00%> (+1.51%) ⬆️
scrapy/utils/trackref.py 85.71% <0.00%> (+2.85%) ⬆️

@kmike kmike merged commit caa1dea into scrapy:master Feb 25, 2020
2 checks passed
@kmike
Copy link
Member

@kmike kmike commented Feb 25, 2020

Thanks @Gallaecio!

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

2 participants