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

Skip scrapy.contracts private APIs in the documentation coverage report #3810

Merged
merged 1 commit into from Jun 13, 2019

Conversation

@Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Jun 4, 2019

I made the following assumptions:

  • Contract’s add_pre_hook and add_post_hook are not documented because they should be transparent to contract developers, for whom pre_hook and post_hook should be the actual concern.
  • ContractsManager is an internal class, developers are not expected to interact with it directly in any way.
  • For default contracts we only want to document their general purpose in their constructor, the methods they reimplement to achieve that purpose should be irrelevant to developers using those contracts.
@kmike
Copy link
Member

@kmike kmike commented Jun 5, 2019

Sounds good @Gallaecio!

What do you think about adding the assumptions you've wrote in a ticket description as comments before the ignore lines?

@Gallaecio Gallaecio changed the title Skip scrapy.contracts private APIs in the documentation coverage report [WIP] Skip scrapy.contracts private APIs in the documentation coverage report Jun 5, 2019
@Gallaecio Gallaecio force-pushed the documentation-coverage branch from dc3d283 to c7ba72b Jun 7, 2019
@codecov
Copy link

@codecov codecov bot commented Jun 7, 2019

Codecov Report

Merging #3810 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3810   +/-   ##
=======================================
  Coverage   85.42%   85.42%           
=======================================
  Files         169      169           
  Lines        9635     9635           
  Branches     1433     1433           
=======================================
  Hits         8231     8231           
  Misses       1156     1156           
  Partials      248      248

@codecov
Copy link

@codecov codecov bot commented Jun 7, 2019

Codecov Report

Merging #3810 into master will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #3810      +/-   ##
=========================================
+ Coverage   85.42%   85.5%   +0.07%     
=========================================
  Files         169     169              
  Lines        9635    9671      +36     
  Branches     1433    1454      +21     
=========================================
+ Hits         8231    8269      +38     
- Misses       1156    1157       +1     
+ Partials      248     245       -3
Impacted Files Coverage Δ
scrapy/loader/__init__.py 96.7% <0%> (+2.18%) ⬆️

@Gallaecio Gallaecio changed the title [WIP] Skip scrapy.contracts private APIs in the documentation coverage report Skip scrapy.contracts private APIs in the documentation coverage report Jun 7, 2019
@dangra dangra merged commit 8a022ac into scrapy:master Jun 13, 2019
3 checks passed
@kmike kmike added this to the v1.7 milestone Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants