Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(13)

#26058: Add dict.__version__ read-only property

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years ago by vstinner
Modified:
3 years, 4 months ago
Reviewers:
jimjjewett, brett
CC:
lemburg, brett.cannon, AntoinePitrou, haypo, r.david.murray, devnull_psf.upfronthosting.co.za, Yury Selivanov, jstasiak
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Total comments: 11

Patch Set 7 #

Total comments: 6

Patch Set 8 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Include/dictobject.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
Lib/test/test_ordered_dict.py View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
Lib/test/test_pep509.py View 1 2 3 4 5 6 7 1 chunk +186 lines, -0 lines 0 comments Download
Lib/test/test_sys.py View 1 2 3 4 5 6 7 2 chunks +3 lines, -3 lines 0 comments Download
Modules/_testcapimodule.c View 1 2 3 4 5 6 7 2 chunks +21 lines, -0 lines 0 comments Download
Objects/dictobject.c View 1 2 3 4 5 6 7 9 chunks +22 lines, -0 lines 0 comments Download

Messages

Total messages: 7
Jim.J.Jewett
http://bugs.python.org/review/26058/diff/16414/Include/dictobject.h File Include/dictobject.h (right): http://bugs.python.org/review/26058/diff/16414/Include/dictobject.h#newcode26 Include/dictobject.h:26: PY_UINT64_T ma_version; Why add this here, instead of after ...
3 years, 12 months ago #1
victor.stinner_gmail.com
Thanks for the review ;-) https://bugs.python.org/review/26058/diff/16414/Include/dictobject.h File Include/dictobject.h (right): https://bugs.python.org/review/26058/diff/16414/Include/dictobject.h#newcode26 Include/dictobject.h:26: PY_UINT64_T ma_version; On 2016/01/26 ...
3 years, 12 months ago #2
victor.stinner_gmail.com
https://bugs.python.org/review/26058/diff/16414/Objects/dictobject.c File Objects/dictobject.c (right): https://bugs.python.org/review/26058/diff/16414/Objects/dictobject.c#newcode217 Objects/dictobject.c:217: #define DICT_NEXT_VERSION() (++pydict_global_version ) useless space :-p
3 years, 9 months ago #3
brett.cannon
LGTM overall. http://bugs.python.org/review/26058/diff/16971/Include/dictobject.h File Include/dictobject.h (right): http://bugs.python.org/review/26058/diff/16971/Include/dictobject.h#newcode26 Include/dictobject.h:26: PY_UINT64_T ma_version_tag; Does putting this in the ...
3 years, 9 months ago #4
victor.stinner_gmail.com
https://bugs.python.org/review/26058/diff/16971/Include/dictobject.h File Include/dictobject.h (right): https://bugs.python.org/review/26058/diff/16971/Include/dictobject.h#newcode26 Include/dictobject.h:26: PY_UINT64_T ma_version_tag; On 2016/04/15 23:40:33, brett.cannon wrote: > Does ...
3 years, 9 months ago #5
brett.cannon
On 2016/04/15 23:47:34, haypo wrote: > https://bugs.python.org/review/26058/diff/16971/Include/dictobject.h > File Include/dictobject.h (right): > > https://bugs.python.org/review/26058/diff/16971/Include/dictobject.h#newcode26 > ...
3 years, 9 months ago #6
victor.stinner_gmail.com
3 years, 4 months ago #7
http://bugs.python.org/review/26058/diff/16971/Lib/test/test_pep509.py
File Lib/test/test_pep509.py (right):

http://bugs.python.org/review/26058/diff/16971/Lib/test/test_pep509.py#newcode29
Lib/test/test_pep509.py:29: def check_version_changed(self, d):
On 2016/04/15 23:40:33, brett.cannon wrote:
> Since you want to test a single action per version change, would it make sense
> to have this take a callable and its arguments to make sure you don't
> accidentally do two mutations at once in the tests and forget to check the
> version changed for each mutation?

Done.

http://bugs.python.org/review/26058/diff/16971/Lib/test/test_pep509.py#newcode36
Lib/test/test_pep509.py:36: self.check_version_changed(d)
On 2016/04/15 23:40:33, brett.cannon wrote:
> Should you just have a method that returns an instance **and** registers the
> initial version in seen_versions? It can even do the check that the creation
> actually did have a new value since that's implicit in the design (unless the
> version({}) == 0 change goes in).

Done.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+