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

gh-94584: Clarify tomllib docs #94585

Closed
wants to merge 3 commits into from
Closed

gh-94584: Clarify tomllib docs #94585

wants to merge 3 commits into from

Conversation

kj7rrv
Copy link
Contributor

@kj7rrv kj7rrv commented Jul 5, 2022

Closes gh-94584

@kj7rrv kj7rrv requested a review from encukou as a code owner July 5, 2022 20:16
@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir labels Jul 5, 2022
Copy link
Contributor

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

This seems unnecessary as there is no real change to the text other than using Deserialize vs. Read and Load. Although technically correct I'm not even sure if it improves the readability that much.

This doesn't need a news entry.

@kj7rrv
Copy link
Contributor Author

kj7rrv commented Jul 6, 2022

@DanielNoord I made a few other changes, e.g.I combined a few short sentences into longer ones. I removed the NEWS entry in 3971067.

@DanielNoord
Copy link
Contributor

In general such word smithing is not really necessary or desired. As you can see there is quite a backlog of open PRs so the time of maintainers is often best spent on meaningful changes to both code and documentation, rather than on sentences that essentially convey the same information.

@kj7rrv
Copy link
Contributor Author

kj7rrv commented Jul 6, 2022

Okay. Should I close this?

@DanielNoord
Copy link
Contributor

I think so, also based on the 👍 my other comment received.

You can look for issues with the easy label if you want to find an issue that might be a good starting point for contributing to the project!

@kj7rrv kj7rrv closed this Jul 6, 2022
@merwok
Copy link
Member

merwok commented Jul 6, 2022

We do appreciate your efforts to contribute! If you find something that interests you in the list of issues, and if you’re not sure about some details, feel free to tag me (or ask on the core-mentorship mailing list if you prefer a more private channel).

@DanielNoord
Copy link
Contributor

@merwok Is there any documentations or tips about attracting core reviewers to your PRs? I know reviews are in high demand and I don't want to spam tags/pings on my PRs so I wonder what the best way to generate some "buzz" for you PR is.

@merwok
Copy link
Member

merwok commented Jul 6, 2022

Some topics have labels with people notified, others may be in the codeowners file, but I don’t know of any system or documentation other than https://devguide.python.org/pullrequest/#reviewing. You get lucky or you wait a bit then send a message on python discuss (the devguide mentions python-dev but discuss also has a core devel section).

Here I am making a specific offer to help one new contributor!

@kj7rrv
Copy link
Contributor Author

kj7rrv commented Jul 6, 2022

@merwok I've opened a few other PRs, #94583 #94611 and #94612. I'm not sure I did #94612 correctly, because the issue it fixes has been open since 2009. I did check and the bug still exists, so I made a PR. Does it look good?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve tomllib docs
5 participants