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

CI: add docformatter to precommit #2614

Merged
merged 19 commits into from Mar 3, 2024
Merged

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Jul 7, 2023

let's get unify the docstrings

cc: @EnricoMi

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.71%. Comparing base (b542438) to head (c433f82).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2614   +/-   ##
=======================================
  Coverage   96.71%   96.71%           
=======================================
  Files         147      147           
  Lines       14885    14885           
=======================================
  Hits        14396    14396           
  Misses        489      489           

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

.pre-commit-config.yaml Show resolved Hide resolved
@Borda Borda requested a review from trim21 July 12, 2023 14:49
@Borda
Copy link
Contributor Author

Borda commented Jul 17, 2023

@sfdye @EnricoMi could you pls have look / review 🦩
Happy to resolve the conflict as I will accept all from the master and just re-run recommit...

Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

I general, the formatting changes look good to me, but there is some unexpected spelling change.

github/RepositoryAdvisoryVulnerabilityPackage.py Outdated Show resolved Hide resolved
Comment on lines 2394 to 2395
: : class:`null <github.GithubObject.NotSet>`:, 'commit'
: : class:`Commit <github.Commit.Commit>`}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, this also seems wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I think in this case the docstring shall be marked with r to be parsed as RST

@Borda Borda requested a review from EnricoMi July 19, 2023 09:38
@Borda Borda marked this pull request as draft July 25, 2023 19:32
@trim21
Copy link
Contributor

trim21 commented Jul 25, 2023

sorry, wrong reply

@trim21
Copy link
Contributor

trim21 commented Jul 25, 2023

image

if the line break in url is not a problem, looks acceptable to me

@Borda
Copy link
Contributor Author

Borda commented Jul 25, 2023

if the line break in url is not a problem, looks acceptable to me

Seems like your message is unrelated to actual state, please update it accordingly

@trim21
Copy link
Contributor

trim21 commented Jul 25, 2023

image
if the line break in url is not a problem, looks acceptable to me

Seems like your message is unrelated to actual state, please update it accordingly

I'm running pre-commit on all files at latest commit b30ea57 of this PR.

@Borda
Copy link
Contributor Author

Borda commented Jul 25, 2023

I'm running pre-commit on all files at latest commit b30ea57 of this PR.

Cool and @trim21 have you noticed the PR status?

@Borda
Copy link
Contributor Author

Borda commented Sep 14, 2023

@EnricoMi applied after all the pyi were merged and all seems fine now, pls check it again :)

@Borda Borda marked this pull request as ready for review September 14, 2023 21:07
@Borda
Copy link
Contributor Author

Borda commented Sep 26, 2023

@EnricoMi mind pls checking again 🙏

@Borda
Copy link
Contributor Author

Borda commented Nov 14, 2023

@EnricoMi is there anything I can do to help it land? 🦩

github/MainClass.py Outdated Show resolved Hide resolved
github/ApplicationOAuth.py Outdated Show resolved Hide resolved
@EnricoMi EnricoMi changed the title precommit: add docformatter CI: add docformatter to precommit Nov 15, 2023
@Borda Borda requested a review from EnricoMi February 1, 2024 22:22
Borda and others added 4 commits February 4, 2024 23:17
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
@Borda Borda requested a review from EnricoMi February 5, 2024 09:21
@Borda
Copy link
Contributor Author

Borda commented Feb 9, 2024

@EnricoMi mind kindly review again, pls 🙏

@EnricoMi
Copy link
Collaborator

Please rebase and rerun again.

@Borda
Copy link
Contributor Author

Borda commented Feb 22, 2024

Please rebase and rerun again.

@EnricoMi merged the main and rerun pre-commit with no changes
our you really need to rebase it? 🦩

@EnricoMi EnricoMi merged commit 96ad19a into PyGithub:main Mar 3, 2024
15 checks passed
@Borda Borda deleted the precommit/docformatter branch March 3, 2024 18:03
@Borda
Copy link
Contributor Author

Borda commented Mar 3, 2024

@EnricoMi thank you, it was long and really appreciate your patience =)

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

Successfully merging this pull request may close these issues.

None yet

4 participants