Add more information to contributing page #7916

Dreamsorcerer · 2023-11-27T17:24:04Z

Mostly revamp coverage section with an example demonstrating how to analyse covecov reports.

I think #7829 works as a perfect example of how codecov is really useful to us.

Preview: https://aiohttp--7916.org.readthedocs.build/en/7916/contributing.html#tests-coverage

docs/contributing.rst

codecov · 2023-11-27T17:37:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (69bbe87) 97.48% compared to head (c949f75) 97.48%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7916   +/-   ##
=======================================
  Coverage   97.48%   97.48%           
=======================================
  Files         107      107           
  Lines       32600    32600           
  Branches     3800     3800           
=======================================
  Hits        31781    31781           
  Misses        616      616           
  Partials      203      203

Flag	Coverage Δ
CI-GHA	`97.40% <ø> (ø)`
OS-Linux	`97.07% <ø> (ø)`
OS-Windows	`95.57% <ø> (ø)`
OS-macOS	`96.88% <ø> (ø)`
Py-3.10.11	`95.50% <ø> (ø)`
Py-3.10.13	`96.87% <ø> (ø)`
Py-3.11.7	`96.56% <ø> (ø)`
Py-3.12.1	`96.69% <ø> (ø)`
Py-3.8.10	`95.47% <ø> (ø)`
Py-3.8.18	`96.81% <ø> (ø)`
Py-3.9.13	`95.47% <ø> (ø)`
Py-3.9.18	`96.84% <ø> (ø)`
Py-pypy7.3.15	`96.40% <ø> (ø)`
VM-macos	`96.88% <ø> (ø)`
VM-ubuntu	`97.07% <ø> (ø)`
VM-windows	`95.57% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

docs/contributing.rst

docs/contributing-cov-header.png

docs/contributing.rst

docs/spelling_wordlist.txt

docs/contributing.rst

Co-authored-by: Sviatoslav Sydorenko <sviat@redhat.com>

docs/contributing.rst

patchback · 2024-01-28T14:56:47Z

Backport to 3.9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.9/822fbc7431f3c5522d3e587ad0b658bef8b6a0ab/pr-7916

Backported as #8072

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Mostly revamp coverage section with an example demonstrating how to analyse covecov reports. --------- Co-authored-by: Sviatoslav Sydorenko <sviat@redhat.com> (cherry picked from commit 822fbc7)

patchback · 2024-01-28T14:56:54Z

Backport to 3.10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.10/822fbc7431f3c5522d3e587ad0b658bef8b6a0ab/pr-7916

Backported as #8073

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Mostly revamp coverage section with an example demonstrating how to analyse covecov reports. --------- Co-authored-by: Sviatoslav Sydorenko <sviat@redhat.com> (cherry picked from commit 822fbc7)

…g page (#8072) **This is a backport of PR #7916 as merged into master (822fbc7).** Co-authored-by: Sam Bull <git@sambull.org>

…ng page (#8073) **This is a backport of PR #7916 as merged into master (822fbc7).** Co-authored-by: Sam Bull <git@sambull.org>

webknjaz · 2024-01-28T16:21:07Z

docs/contributing.rst

+
+.. tip::
+   Sometimes the diff on *codecov.io* doesn't make sense. This is usually
+   caused by the branch being out of sync with master. Try merging


@Dreamsorcerer this isn't exactly the case I meant. What I'm talking about is GHA attempting to upload coverage and getting HTTP 500 from Codecov API in some or all of the jobs. Sometimes, restarting those jobs doesn't help and it just keeps erroring out for hours.
This means that part of the coverage uploads happened from some envs, but not all. And depending on which ones failed, the coverage may show up lower than it actually is.

To make things worse, by default, the GHA step doing the upload succeeds silently. And the only way to discover if some jobs didn't actually upload coverage is to click through each CI job, expand that step and read the log (it'll either have a URL of the upload for the current commit/PR or some error message at the end of the console output).

What I'm talking about is GHA attempting to upload coverage and getting HTTP 500 from Codecov API in some or all of the jobs.

Oh, I think I enabled the option to fail CI if an error occurs in upload (unless it was a different repo), so we shouldn't get that happening anymore.

Different repo, probably. But still, I'm not sure if that option is what we want — it effectively marks entire jobs as failing. And whenever Codecov has platform issues, that means fully blocking merges for hours if not days.

Problem is that we've had several repos which have been failing coverage uploads for a long time, due to configuration issues. I've personally not seen codecov fail on upload as an issue on their end yet.

I've definitely seen some. Both in aio-libs and other places. It's sometimes flaky on their platform side. And restarts result in HTTP 500, then in several hours it just works when restarted. And I observed these outages affecting repos in different orgs in the same time frame, while their status pages happily reported "no platform issues".

And that is the primary reason https://hynek.me/articles/ditch-codecov-python/ is a thing.
I actually think, parts of that article should be implemented here too. Maybe, not in place of Codecov but additionally to it.

P.S. In the past I also blamed misconfiguration a lot of times. But that doesn't account for when its not the case.

webknjaz · 2024-01-28T23:56:21Z

CHANGES/README.rst

@@ -1,15 +1,23 @@
-.. _Adding change notes with your PRs:


Changing this label is going to be problematic as other projects rely on it for referring the changelog guidelines from other places. Not all the PR recommendations set. It'll have to be reverted.

webknjaz · 2024-01-28T23:58:23Z

CHANGES/README.rst

-.. _Adding change notes with your PRs:
+.. _Making a pull request:
+
+Making a pull request


@Dreamsorcerer this title was a mistake:

Since it's rendered on GitHub in a folder with change notes, on any branch, it shouldn't really mention the PR process that much — just the process for having the changelog.

The Sphinx site now has 2 subsequent titles with the same name but different capitalization — https://docs.aiohttp.org/en/stable/contributing.html#making-a-pull-request and https://docs.aiohttp.org/en/stable/contributing.html#id2.

webknjaz · 2024-01-28T23:59:29Z

CHANGES/README.rst

+
+When making a pull request, please include a short summary of the changes
+and a reference to any issue tickets that the PR is intended to solve.
+All PRs with code changes should include tests. All changes should


Yeah, I missed that this now refers to tests that are not related to the https://github.com/aio-libs/aiohttp/tree/master/CHANGES#readme directory at all.

…#7916)" This partially reverts commit 822fbc7. In particular, this drops the top level title from README in the `CHANGES/` folder and restores the original label. For the proper Sphinx ToC structuring, a title in the `docs/contributing.rst` document.

…tion to contributing page (#7916)" (#8109) **This is a backport of PR #8107 as merged into master (854e6d8).** This partially reverts commit 822fbc7. In particular, this drops the top level title from README in the `CHANGES/` folder and restores the original label. For the proper Sphinx ToC structuring, a title in the `docs/contributing.rst` document. Co-authored-by: Sviatoslav Sydorenko <wk@sydorenko.org.ua>

…ion to contributing page (#7916)" (#8108) **This is a backport of PR #8107 as merged into master (854e6d8).** This partially reverts commit 822fbc7. In particular, this drops the top level title from README in the `CHANGES/` folder and restores the original label. For the proper Sphinx ToC structuring, a title in the `docs/contributing.rst` document. Co-authored-by: Sviatoslav Sydorenko <wk@sydorenko.org.ua>

webknjaz · 2024-01-30T02:54:35Z

docs/contributing.rst

@@ -220,47 +260,19 @@ To run spell checker on Linux box you should install it first:
   $ sudo apt-get install enchant
   $ pip install sphinxcontrib-spelling

-Changelog update


@Dreamsorcerer I think you had Sphinx problems because you didn't keep this title as in examples. I applied a partial revert and there's no errors.

(cherry picked from commit 2d8ffdf)

… PR #7916 (#8112) **This is a backport of PR #8110 as merged into master (2d8ffdf).** None Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <sviat@redhat.com>

#7916 (#8111) **This is a backport of PR #8110 as merged into master (2d8ffdf).** None Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <sviat@redhat.com>

Add more information to contributing page.

d155e00

Dreamsorcerer added backport-3.9 backport-3.10 Trigger automatic backporting to the 3.10 release branch by Patchback robot labels Nov 27, 2023

Dreamsorcerer requested review from webknjaz and bdraco November 27, 2023 17:24

Dreamsorcerer requested a review from asvetlov as a code owner November 27, 2023 17:24

Create 7916.doc

1abf034

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Nov 27, 2023

Dreamsorcerer commented Nov 27, 2023

View reviewed changes

docs/contributing.rst Outdated Show resolved Hide resolved

Update docs/contributing.rst

616576b

bdraco reviewed Nov 27, 2023

View reviewed changes

docs/contributing.rst Outdated Show resolved Hide resolved

bdraco reviewed Nov 27, 2023

View reviewed changes

docs/contributing.rst Outdated Show resolved Hide resolved

bdraco approved these changes Nov 27, 2023

View reviewed changes

Dreamsorcerer added 3 commits November 27, 2023 17:47

Update contributing.rst

1b67ffa

Update 7916.doc

4677136

Update spelling_wordlist.txt

37a5a3a

webknjaz reviewed Nov 29, 2023

View reviewed changes

docs/contributing-cov-header.png Outdated Show resolved Hide resolved

webknjaz reviewed Nov 29, 2023

View reviewed changes

docs/contributing.rst Outdated Show resolved Hide resolved

webknjaz reviewed Nov 29, 2023

View reviewed changes

docs/contributing.rst Show resolved Hide resolved

webknjaz reviewed Nov 29, 2023

View reviewed changes

docs/spelling_wordlist.txt Outdated Show resolved Hide resolved

webknjaz reviewed Nov 29, 2023

View reviewed changes

docs/contributing.rst Outdated Show resolved Hide resolved

Dreamsorcerer and others added 8 commits December 1, 2023 17:19

Update docs/contributing.rst

d4f956b

Co-authored-by: Sviatoslav Sydorenko <sviat@redhat.com>

Include changes readme

aa58588

Update README.rst

a2a6ed9

Update contributing.rst

c8d8887

Update spelling_wordlist.txt

d625b3d

Update contributing.rst

8c3d928

Update README.rst

14acacf

Update README.rst

c761044

Dreamsorcerer added 3 commits January 28, 2024 14:27

Delete docs/contributing-cov-partial.png

aeb4018

Update contributing.rst

8284d87

Merge branch 'master' into docs-contributing

1310a6d

Dreamsorcerer commented Jan 28, 2024

View reviewed changes

docs/contributing.rst Outdated Show resolved Hide resolved

Update docs/contributing.rst

c949f75

Dreamsorcerer enabled auto-merge (squash) January 28, 2024 14:49

Dreamsorcerer merged commit 822fbc7 into master Jan 28, 2024
28 of 30 checks passed

Dreamsorcerer deleted the docs-contributing branch January 28, 2024 14:56

patchback bot mentioned this pull request Jan 28, 2024

[PR #7916/822fbc74 backport][3.9] Add more information to contributing page #8072

Merged

patchback bot mentioned this pull request Jan 28, 2024

[PR #7916/822fbc74 backport][3.10] Add more information to contributing page #8073

Merged

Dreamsorcerer added a commit that referenced this pull request Jan 28, 2024

[PR #7916/822fbc74 backport][3.9] Add more information to contributin…

0d945d1

…g page (#8072) **This is a backport of PR #7916 as merged into master (822fbc7).** Co-authored-by: Sam Bull <git@sambull.org>

Dreamsorcerer added a commit that referenced this pull request Jan 28, 2024

[PR #7916/822fbc74 backport][3.10] Add more information to contributi…

d445803

…ng page (#8073) **This is a backport of PR #7916 as merged into master (822fbc7).** Co-authored-by: Sam Bull <git@sambull.org>

webknjaz reviewed Jan 28, 2024

View reviewed changes

webknjaz added a commit to webknjaz/aiohttp that referenced this pull request Jan 30, 2024

Re-add a PR preparation intro from PR aio-libs#7916

b7ff45a

webknjaz reviewed Jan 30, 2024

View reviewed changes

webknjaz added a commit that referenced this pull request Jan 30, 2024

Re-add a PR preparation intro from PR #7916 (#8110)

2d8ffdf

patchback bot pushed a commit that referenced this pull request Jan 30, 2024

Re-add a PR preparation intro from PR #7916 (#8110)

fbd5399

(cherry picked from commit 2d8ffdf)

patchback bot pushed a commit that referenced this pull request Jan 30, 2024

Re-add a PR preparation intro from PR #7916 (#8110)

4efeffb

(cherry picked from commit 2d8ffdf)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more information to contributing page #7916

Add more information to contributing page #7916

Dreamsorcerer commented Nov 27, 2023 •

edited

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

patchback bot commented Jan 28, 2024 •

edited

Loading

patchback bot commented Jan 28, 2024 •

edited

Loading

webknjaz Jan 28, 2024

Dreamsorcerer Jan 28, 2024

webknjaz Jan 28, 2024

Dreamsorcerer Jan 28, 2024

webknjaz Jan 30, 2024

webknjaz Jan 28, 2024

webknjaz Jan 28, 2024

webknjaz Jan 28, 2024

webknjaz Jan 30, 2024

Add more information to contributing page #7916

Add more information to contributing page #7916

Conversation

Dreamsorcerer commented Nov 27, 2023 • edited Loading

codecov bot commented Nov 27, 2023 • edited Loading

Codecov Report

patchback bot commented Jan 28, 2024 • edited Loading

Backport to 3.9: 💚 backport PR created

patchback bot commented Jan 28, 2024 • edited Loading

Backport to 3.10: 💚 backport PR created

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dreamsorcerer commented Nov 27, 2023 •

edited

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

patchback bot commented Jan 28, 2024 •

edited

Loading

patchback bot commented Jan 28, 2024 •

edited

Loading