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

[MRG+1] Fix numeration #3694

Merged
merged 1 commit into from Mar 22, 2019
Merged

[MRG+1] Fix numeration #3694

merged 1 commit into from Mar 22, 2019

Conversation

@noviluni
Copy link
Member

@noviluni noviluni commented Mar 20, 2019

Fix numeration in selectors documentation.

@codecov
Copy link

@codecov codecov bot commented Mar 20, 2019

Codecov Report

Merging #3694 into master will decrease coverage by 0.17%.
The diff coverage is 88.23%.

@@            Coverage Diff             @@
##           master    #3694      +/-   ##
==========================================
- Coverage   84.72%   84.54%   -0.18%     
==========================================
  Files         168      167       -1     
  Lines        9457     9422      -35     
  Branches     1407     1402       -5     
==========================================
- Hits         8012     7966      -46     
- Misses       1188     1199      +11     
  Partials      257      257
Impacted Files Coverage Δ
scrapy/utils/python.py 82.68% <ø> (ø) ⬆️
scrapy/commands/settings.py 72.72% <ø> (ø) ⬆️
scrapy/selector/lxmlsel.py 88.88% <ø> (ø) ⬆️
scrapy/utils/log.py 88.5% <ø> (ø) ⬆️
scrapy/pipelines/__init__.py 91.66% <ø> (ø) ⬆️
scrapy/spidermiddlewares/referer.py 93.7% <ø> (ø) ⬆️
scrapy/conf.py 85.71% <ø> (ø) ⬆️
scrapy/utils/test.py 54.54% <ø> (ø) ⬆️
scrapy/utils/defer.py 92.98% <ø> (ø) ⬆️
scrapy/core/downloader/handlers/ftp.py 98.24% <ø> (ø) ⬆️
... and 80 more

@Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Mar 22, 2019

Please, keep things simple a separate: have a pull request to fix the numbering that only fixes the numbering and a pull request about the duplicate .gitignore entry that only removes the duplicate .gitignore entry.

As for PEP8, see #2144. I do think that, if done, it should include something (e.g. integration of pytest-pylint) that ensures that breaking PEP8 again results in failed tests.

@noviluni
Copy link
Member Author

@noviluni noviluni commented Mar 22, 2019

Hi Adrián!

Totally agree

My first intention was to push only the first commit, but I was working in other pull requests when accidentally pushed to this branch.

I fixed it. I also pushed .gitignore change to another branch and made a PR (#3698). PEP 8 commit was just a proof of concept to discuss about it.

@Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Mar 22, 2019

That was quick. Thanks!

@Gallaecio Gallaecio changed the title Fix numeration [MRG+1] Fix numeration Mar 22, 2019
@kmike kmike merged commit 8583c03 into scrapy:master Mar 22, 2019
3 checks passed
@kmike
Copy link
Member

@kmike kmike commented Mar 22, 2019

Thanks @noviluni!

@kmike kmike added this to the v1.7 milestone Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants