Skip to content

Grammar fixes to whatsnew/3.8.rst#16621

Closed
akuchling wants to merge 7 commits into
python:masterfrom
akuchling:akuchling-whatsnew-fixes
Closed

Grammar fixes to whatsnew/3.8.rst#16621
akuchling wants to merge 7 commits into
python:masterfrom
akuchling:akuchling-whatsnew-fixes

Conversation

@akuchling

Copy link
Copy Markdown
Contributor

I read through the 3.8 document and fixed various grammar issues. Also, the library modules weren't quite in alphabetical order, and there was a duplicated idlelib/IDLE section (maybe a merging mishap?), so I rearranged them without making any other textual change in that commit. This means the PR is likely easier to read if you go commit-by-commit instead of reading the entire diff.

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting core review labels Oct 7, 2019
@pablogsal pablogsal changed the title Grammar fixes to whatsnew/3.8.rst [3.8] Grammar fixes to whatsnew/3.8.rst Oct 10, 2019
@ned-deily
ned-deily requested review from ambv and rhettinger October 11, 2019 20:45
@ned-deily

Copy link
Copy Markdown
Member

@akuchling, This should be merged into master first and then backported to 3.8 as the 3.8.rst exists both places. (or forward ported from here to master but you'd have to do that manually with cherry-pick)

@taleinat

taleinat commented Oct 11, 2019

Copy link
Copy Markdown
Contributor

(See related bpo-38450.)

Comment thread Doc/whatsnew/3.8.rst Outdated
Comment thread Doc/whatsnew/3.8.rst Outdated
Comment thread Doc/whatsnew/3.8.rst Outdated
Comment thread Doc/whatsnew/3.8.rst Outdated
@bedevere-bot

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@terryjreedy

Copy link
Copy Markdown
Member

I suggested a 4 minor changes but this is otherwise an essential improvement that should be merged before 3.8.0 final even without them.

@rhettinger

Copy link
Copy Markdown
Contributor

This needs to be applied to 3.9, then backported to 3.8. Otherwise, it messes-up our workflow and the other PR that work on 3.8.

@akuchling

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

(I also changed the PR to be against master and not 3.8.)

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@rhettinger, @terryjreedy: please review the changes made to this pull request.

@ned-deily

Copy link
Copy Markdown
Member

I would not try to push to another branch in the existing pull request because you end up with a merge nightmare. I would just open a separate PR for master and copy the files from your 3.8 PR there.

@bedevere-bot
bedevere-bot requested review from rhettinger and terryjreedy and removed request for a team, ezio-melotti, gvanrossum, ilevkivskyi, pablogsal and skrah October 12, 2019 18:38
@pablogsal pablogsal changed the title [3.8] Grammar fixes to whatsnew/3.8.rst Grammar fixes to whatsnew/3.8.rst Oct 12, 2019
@rhettinger

Copy link
Copy Markdown
Contributor

This all looks fine to me. Let's go ahead and get this in, so I can continue to make additions and improvements.

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

only minor comments, the can be addressed later if you want to merge this as is.

Comment thread Doc/whatsnew/3.8.rst
Comment thread Doc/whatsnew/3.8.rst
Comment thread Doc/whatsnew/3.8.rst Outdated
are consumed.
See :ref:`shutil-platform-dependent-efficient-copy-operations` section.
(Contributed by Giampaolo Rodola' in :issue:`33671`.)
(Contributed by Giampaolo Rodola in :issue:`33671`.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The best spelling is actually Rolodà. Can we use Unicode for names at least?

@akuchling

Copy link
Copy Markdown
Contributor Author

I would not try to push to another branch in the existing pull request because you end up with a merge nightmare. I would just open a separate PR for master and copy the files from your 3.8 PR there.

Even if I've rebased the branch to have just my changes atop master? I'd expect the PR should be fine then, no?

@ned-deily

ned-deily commented Oct 13, 2019

Copy link
Copy Markdown
Member

For one thing, all those code-owner reviewers were spuriously added to the PR. There's nothing to be done about that now. In this case, it will probably be OK since it is only one file and we automatically do squash merges but I've found it is best to avoid changing branches with our workflow.

@terryjreedy

terryjreedy commented Oct 13, 2019

Copy link
Copy Markdown
Member

The tradeoff is 5-10 minutes to make a fresh branch and PR from freshly updated master versus the possibility that committing this will cause some problems. I would go with the former. (I am literally on my way to bed. Otherwise, I would offer to do it.) Either way, commit so that Raymond can do a pass over the updated file.

@akuchling

Copy link
Copy Markdown
Contributor Author

Closing in favour of an updated PR against master: #16745

@akuchling akuchling closed this Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants