Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-108202:calendar: DocumentTextCalendar methods#127608

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
erlend-aasland merged 14 commits intopython:mainfromStanFromIreland:patch-4
Jan 3, 2025

Conversation

@StanFromIreland
Copy link
Member

@StanFromIrelandStanFromIreland commentedDec 4, 2024•
edited by github-actionsbot
Loading

@StanFromIrelandStanFromIreland marked this pull request as ready for reviewDecember 5, 2024 16:27
@StanFromIreland

This comment was marked as outdated.

Copy link
Member

@ZeroIntensityZeroIntensity left a comment

Choose a reason for hiding this comment

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

Some initial comments. (I'll tag Erlend again once I consider this ready from a triage standpoint.)

It's probably worth converting the phrase "string representation" into a reference to thestr class.

erlend-aasland reacted with thumbs up emoji
@ZeroIntensityZeroIntensity added needs backport to 3.12only security fixes needs backport to 3.13bugs and security fixes labelsDec 10, 2024
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
As per ZeroIntenisities suggestion
@Antony3293Antony3293 mentioned this pull requestDec 10, 2024
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Copy link
Member

@ZeroIntensityZeroIntensity left a comment

Choose a reason for hiding this comment

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

Ok, I'll refrain from nitpicking. I don't know enough aboutcalendar to give this a green check, so I'll just tag Erlend.

erlend-aasland reacted with laugh emoji
@StanFromIreland
Copy link
MemberAuthor

@erlend-aasland All done now. In the future do you prefer a PR for every function like hugokvs maybe it is easier to review?

@erlend-aasland
Copy link
Contributor

No, do not create one PR for every function. Just don't mix different types of changes in a PR (nor in an issue). This is all explained in clear text in the devguide.

@StanFromIreland
Copy link
MemberAuthor

@erlend-aasland ok. Is this PR ok for you then? Are there any other changes you want before you review? This PR was originally focused on just the functions but then expanded with suggestions from other reviewers, I have reverted all of them.

@erlend-aasland
Copy link
Contributor

Is this PR ok for you then?

I'm torn about these changes. If you take the time to read through this issue's history, you'll find that it is in fact pretty controversial. Some core devs actively oppose documenting methods that could/should be considered helpers and/or private APIs. Other core devs lean towards that every (intentional or unintentional) public API should be documented. There is no happy path.

I guess I'm fine with this; since other similar calendar formatting helpers have been documented, we might as well document the rest of them.

@erlend-aasland
Copy link
Contributor

This PR was originally focused on just the functions but then expanded with suggestions from other reviewers, I have reverted all of them.

I see; I'm sorry about the extra work. I did not read through all of the previous remarks. I prefer to align strictly with our recommended development guidelines and strictly keepany type of change atomic. This implies:

  • not mixing multiple bugfixes
  • not mixing bugfixes and features
  • not mixing multiple features (related or unrelated)
  • not mixing different types of documentation enhancements or "bugfixes"

As with any kind of rule, there will be exceptions. But exceptions areexceptions; if they become the new norm, our guidelines become moot.

Keeping changes atomic has several positive effects:

  • changes are easier to review
  • code changes are easier to debug1
  • backporting is made easier
  • reverting problematic changes is easier

Footnotes

  1. note thatgit bisect becomes pointless if you mix bugfixes and/or features in a single PR↩

@erlend-aasland
Copy link
Contributor

@picnixz: if you are satisfied with this, feel free to land it.

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

A last markup change and we're good.

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@picnixz
Copy link
Member

(FTR, I still don't have the commit rights so you'll still need to hit the merge button@erlend-aasland :'))

@erlend-aasland
Copy link
Contributor

erlend-aasland commentedJan 3, 2025•
edited
Loading

(FTR, I still don't have the commit rights [...]

What?! Outrageous! 😄

picnixz reacted with laugh emoji

@erlend-aaslanderlend-aasland merged commitbb73426 intopython:mainJan 3, 2025
25 checks passed
@miss-islington-app

This comment was marked as outdated.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJan 3, 2025
…ythonGH-127608)Document the following TextCalendar methods:- formatday()- formatmonthname()- formatweekday()- formatweekheader()(cherry picked from commitbb73426)Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJan 3, 2025
…ythonGH-127608)Document the following TextCalendar methods:- formatday()- formatmonthname()- formatweekday()- formatweekheader()(cherry picked from commitbb73426)Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@bedevere-app
Copy link

GH-128440 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelJan 3, 2025
@bedevere-app
Copy link

GH-128441 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelJan 3, 2025
erlend-aasland pushed a commit that referenced this pull requestJan 3, 2025
…H-127608) (#128441)Document the following TextCalendar methods:- formatday()- formatmonthname()- formatweekday()- formatweekheader()(cherry picked from commitbb73426)Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
erlend-aasland pushed a commit that referenced this pull requestJan 3, 2025
…H-127608) (#128440)Document the following TextCalendar methods:- formatday()- formatmonthname()- formatweekday()- formatweekheader()(cherry picked from commitbb73426)Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
WolframAlph pushed a commit to WolframAlph/cpython that referenced this pull requestJan 4, 2025
…ython#127608)Document the following TextCalendar methods:- formatday()- formatmonthname()- formatweekday()- formatweekheader()
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull requestJan 8, 2025
…ython#127608)Document the following TextCalendar methods:- formatday()- formatmonthname()- formatweekday()- formatweekheader()
@StanFromIrelandStanFromIreland deleted the patch-4 branchFebruary 11, 2025 16:42
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@eendebakpteendebakpteendebakpt approved these changes

@picnixzpicnixzpicnixz approved these changes

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes

@ZeroIntensityZeroIntensityAwaiting requested review from ZeroIntensity

Assignees

@picnixzpicnixz

Labels

docsDocumentation in the Doc dirskip news

Projects

Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@StanFromIreland@eendebakpt@ZeroIntensity@hugovk@erlend-aasland@picnixz
[8]ページ先頭
©2009-2025 Movatter.jp