◐ Shell
reader mode source ↗
Skip to content

bpo-28810: Document remaining bytecode changes in 3.6#651

Merged
brettcannon merged 8 commits into
python:masterfrom
ilevkivskyi:more-bytecode-docs
Mar 24, 2017
Merged

bpo-28810: Document remaining bytecode changes in 3.6#651
brettcannon merged 8 commits into
python:masterfrom
ilevkivskyi:more-bytecode-docs

Conversation

@ilevkivskyi

@ilevkivskyi ilevkivskyi commented Mar 13, 2017

Copy link
Copy Markdown
Member

@serhiy-storchaka @brettcannon
These are two remaining changes to close http://bugs.python.org/issue28810
This needs a backport to 3.6

@mention-bot

mention-bot commented Mar 13, 2017

Copy link
Copy Markdown

@ilevkivskyi, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @ncoghlan, @benjaminp, @serhiy-storchaka and @1st1 to be potential reviewers.

@serhiy-storchaka serhiy-storchaka added type-feature A feature request or enhancement needs backport to 3.6 docs Documentation in the Doc dir and removed type-feature A feature request or enhancement labels Mar 13, 2017
@ilevkivskyi

ilevkivskyi commented Mar 13, 2017

Copy link
Copy Markdown
Member Author

@serhiy-storchaka Thanks!

I think this should be moved to the top of the document, under impl-detail block

I was also thinking about this. I implemented both your comments in a new commit.

@ilevkivskyi

ilevkivskyi commented Mar 13, 2017

Copy link
Copy Markdown
Member Author

By the way, I have noticed that the example in Objects/lnotab_notes.txt is outdated. It still uses Python 2 and old bytecode (not 2 bytes per instruction).

Should I update it in this PR or open a separate issue and make a separate PR?

@brettcannon

brettcannon commented Mar 13, 2017

Copy link
Copy Markdown
Member

@ilevkivskyi separate PR would be great!

@ilevkivskyi

ilevkivskyi commented Mar 13, 2017

Copy link
Copy Markdown
Member Author

separate PR would be great!

OK, will do this.
So it looks like this PR could be merged now.

serhiy-storchaka
serhiy-storchaka reviewed Mar 13, 2017
View reviewed changes

@serhiy-storchaka serhiy-storchaka 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.

Hide comment

The meaning of HAVE_ARGUMENT was changed too. Now all instructions have an argument, but opcodes < HAVE_ARGUMENT ignore it.

@ilevkivskyi

ilevkivskyi commented Mar 14, 2017

Copy link
Copy Markdown
Member Author

@serhiy-storchaka Thanks, I implemented your comments.

serhiy-storchaka
serhiy-storchaka approved these changes Mar 14, 2017
View reviewed changes

@serhiy-storchaka serhiy-storchaka 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.

Hide comment

I don't know whether the changes use good English, but in general LGTM.

@ilevkivskyi

ilevkivskyi commented Mar 18, 2017

Copy link
Copy Markdown
Member Author

@brettcannon Thanks! I fixed both points in a new commit.

@brettcannon brettcannon self-assigned this Mar 20, 2017
@brettcannon brettcannon merged commit 8f9e1bb into python:master Mar 24, 2017
brettcannon pushed a commit to brettcannon/cpython that referenced this pull request Mar 24, 2017
@ilevkivskyi @brettcannon
bpo-28810: Document remaining bytecode changes in 3.6 (pythonGH-651) …
81230f9 
(cherry picked from commit 8f9e1bb)
brettcannon added a commit that referenced this pull request Mar 24, 2017
@brettcannon
bpo-28810: Document remaining bytecode changes in 3.6 (GH-651) (GH-808) …
74bfcc3 
(cherry picked from commit 8f9e1bb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments

@ncoghlan ncoghlan ncoghlan left review comments

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@brettcannon brettcannon brettcannon approved these changes

Assignees

@brettcannon brettcannon

Labels

docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ilevkivskyi @mention-bot @brettcannon @vstinner @ncoghlan @serhiy-storchaka @the-knights-who-say-ni