)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"e25c813651250c4eef020f866cbcde362a3f98e0","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make discover._version_between more lenient"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In response to [1], discover._version_between now allows un-normalized"},{"line_number":10,"context_line":"input, and responds sensibly to unspecified upper \u0026 lower bounds."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1] https://review.openstack.org/#/c/483604/9/keystoneauth1/discover.py@306"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ff346bd7_a0fe8234","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":17},"updated":"2017-07-25 21:04:32.000000000","message":"I would kind of rather this not be just a response to my complaint: my complaint was, \"this seems unrelated to the rest of the change, and it\u0027s unexplained.\" If there is good reason for it to be the way it is, please explain it to me and let\u0027s keep it. If it was an arbitrary change and you agree that this current proposal is the right way to go, then please explain why this way is better in this commit message.","commit_id":"ae9b1a08a17268eb4aa1c4268fedbcaa69c4ed6d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8ed5369e73bfca0820404117b43eee69324091c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make discover._version_between more lenient"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In response to [1], discover._version_between now allows un-normalized"},{"line_number":10,"context_line":"input, and responds sensibly to unspecified upper \u0026 lower bounds."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"[1] https://review.openstack.org/#/c/483604/9/keystoneauth1/discover.py@306"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ff346bd7_5bb90bca","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":17},"in_reply_to":"ff346bd7_a0fe8234","updated":"2017-07-25 21:29:14.000000000","message":"Fair.  I\u0027ll take a closer look with that in mind.","commit_id":"ae9b1a08a17268eb4aa1c4268fedbcaa69c4ed6d"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"4e80bbf1ed391c5e13964d6d1813bd04e8fad320","unresolved":false,"context_lines":[{"line_number":23,"context_line":"chart."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"This change allows us to explain clearly what happens when upper or"},{"line_number":26,"context_line":"lower bounds are None: it translates naturally to \"there is no bound\"."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Given that, the first line of the chart (None/None) now always returns"},{"line_number":29,"context_line":"True, which is a behavior change the callers need to be updated to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7f287b81_dbbf5fa8","line":26,"updated":"2017-09-04 19:52:42.000000000","message":"I get this in theory but in practice this seems to just mean we\u0027re moving the bound check to outside of the method when it might as well stay inside.","commit_id":"1de528ece288e24abb627642af6d1557e2a01dcb"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"199252baa4d29889f7373e954a3011df10a1fd55","unresolved":false,"context_lines":[{"line_number":23,"context_line":"chart."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"This change allows us to explain clearly what happens when upper or"},{"line_number":26,"context_line":"lower bounds are None: it translates naturally to \"there is no bound\"."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Given that, the first line of the chart (None/None) now always returns"},{"line_number":29,"context_line":"True, which is a behavior change the callers need to be updated to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7f287b81_23562a08","line":26,"in_reply_to":"7f287b81_dbbf5fa8","updated":"2017-09-05 11:53:51.000000000","message":"Moving the \"no bound\" check.  But only the one that corresponds to the first row in the above table.  Which is why it was confusing in the old version, and why I wanted to change it.  There\u0027s weirdness around the logic no matter what; this delta isolates the weirdness to where it is used.  I feel like a small helper method called \"version between\" should and can be really easy to understand, and the base rev here just ain\u0027t.\n\nBut look, I\u0027m not stuck on this change set.  If I haven\u0027t convinced you thus far, then I\u0027m sure it just boils down to a subjective/aesthetic difference, and I\u0027m okay abandoning.","commit_id":"1de528ece288e24abb627642af6d1557e2a01dcb"}],"keystoneauth1/discover.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5b4b9fa9667c83f1e31c7f9a7c4843a18d1b7c6d","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                return data"},{"line_number":609,"context_line":"            if _latest_soft_match(min_version, data[\u0027version\u0027]):"},{"line_number":610,"context_line":"                return data"},{"line_number":611,"context_line":"            if min_version and max_version and _version_between("},{"line_number":612,"context_line":"                    min_version, max_version, data[\u0027version\u0027]):"},{"line_number":613,"context_line":"                return data"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_1c41730c","line":611,"updated":"2017-07-24 15:18:51.000000000","message":"I don\u0027t like having to do this.\n\nBut I can\u0027t justify _version_between(None, None, anything) returning False given that None acts as \"no bound\" if the other one is specified.\n\nThoughts?","commit_id":"7be6c04e0a850f1a0ac8944501a9179a9c0ed074"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"048cb2f4fd3357fc0e85f12771d02ce15b936530","unresolved":false,"context_lines":[{"line_number":278,"context_line":"def _version_between(min_version, max_version, candidate):"},{"line_number":279,"context_line":"    \"\"\"Determine whether a candidate version is within a specified range."},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    :param min_version: Lower bound, in any normalize_version_number-able form."},{"line_number":282,"context_line":"                        None/empty indicates no lower bound."},{"line_number":283,"context_line":"    :param max_version: Upper bound, in any normalize_version_number-able form."},{"line_number":284,"context_line":"                        None/empty indicates no upper bound."},{"line_number":285,"context_line":"    :param candidate: Candidate version to test, in any form suitable for"},{"line_number":286,"context_line":"                      normalize_version_number.  May not be None/empty."},{"line_number":287,"context_line":"    :return: True if candidate is between min_version and max_version; False"},{"line_number":288,"context_line":"             otherwise."},{"line_number":289,"context_line":"    :raises ValueError: If candidate is None or the input is not properly"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_27cbe572","line":286,"range":{"start_line":281,"start_character":4,"end_line":286,"end_character":71},"updated":"2017-07-25 08:02:21.000000000","message":"Just use the same description that the other version parameters use, e.g. http://git.openstack.org/cgit/openstack/keystoneauth/tree/keystoneauth1/identity/base.py#n206\n\nnormalize_version_number-able is meaningless to a user, and the point of changing this interface is to make the input flexible and the normalization process transparent.","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c9910f4634ef1deecfde77e39b7a22cb823751f2","unresolved":false,"context_lines":[{"line_number":278,"context_line":"def _version_between(min_version, max_version, candidate):"},{"line_number":279,"context_line":"    \"\"\"Determine whether a candidate version is within a specified range."},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    :param min_version: Lower bound, in any normalize_version_number-able form."},{"line_number":282,"context_line":"                        None/empty indicates no lower bound."},{"line_number":283,"context_line":"    :param max_version: Upper bound, in any normalize_version_number-able form."},{"line_number":284,"context_line":"                        None/empty indicates no upper bound."},{"line_number":285,"context_line":"    :param candidate: Candidate version to test, in any form suitable for"},{"line_number":286,"context_line":"                      normalize_version_number.  May not be None/empty."},{"line_number":287,"context_line":"    :return: True if candidate is between min_version and max_version; False"},{"line_number":288,"context_line":"             otherwise."},{"line_number":289,"context_line":"    :raises ValueError: If candidate is None or the input is not properly"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_3570a1dc","line":286,"range":{"start_line":281,"start_character":4,"end_line":286,"end_character":71},"in_reply_to":"ff346bd7_27cbe572","updated":"2017-07-25 15:09:47.000000000","message":"Done","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"048cb2f4fd3357fc0e85f12771d02ce15b936530","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                      normalize_version_number.  May not be None/empty."},{"line_number":287,"context_line":"    :return: True if candidate is between min_version and max_version; False"},{"line_number":288,"context_line":"             otherwise."},{"line_number":289,"context_line":"    :raises ValueError: If candidate is None or the input is not properly"},{"line_number":290,"context_line":"                        normalized."},{"line_number":291,"context_line":"    :raises TypeError: If any input cannot be normalized."},{"line_number":292,"context_line":"    \"\"\""},{"line_number":293,"context_line":"    if not candidate:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_e7c4bd61","line":290,"range":{"start_line":289,"start_character":44,"end_line":290,"end_character":35},"updated":"2017-07-25 08:02:21.000000000","message":"not true anymore","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c9910f4634ef1deecfde77e39b7a22cb823751f2","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                      normalize_version_number.  May not be None/empty."},{"line_number":287,"context_line":"    :return: True if candidate is between min_version and max_version; False"},{"line_number":288,"context_line":"             otherwise."},{"line_number":289,"context_line":"    :raises ValueError: If candidate is None or the input is not properly"},{"line_number":290,"context_line":"                        normalized."},{"line_number":291,"context_line":"    :raises TypeError: If any input cannot be normalized."},{"line_number":292,"context_line":"    \"\"\""},{"line_number":293,"context_line":"    if not candidate:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_1577ddd6","line":290,"range":{"start_line":289,"start_character":44,"end_line":290,"end_character":35},"in_reply_to":"ff346bd7_e7c4bd61","updated":"2017-07-25 15:09:47.000000000","message":"Done","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"048cb2f4fd3357fc0e85f12771d02ce15b936530","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                return data"},{"line_number":609,"context_line":"            if _latest_soft_match(min_version, data[\u0027version\u0027]):"},{"line_number":610,"context_line":"                return data"},{"line_number":611,"context_line":"            if min_version and max_version and _version_between("},{"line_number":612,"context_line":"                    min_version, max_version, data[\u0027version\u0027]):"},{"line_number":613,"context_line":"                return data"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_e7439dae","line":611,"updated":"2017-07-25 08:02:21.000000000","message":"Before the change that restricted the inputs, we used to allow min_version and max_version to be None and interpreted them to mean \u0027latest\u0027: http://git.openstack.org/cgit/openstack/keystoneauth/tree/keystoneauth1/discover.py?id\u003d218adc333efe0cea8f01ee11666e53f4c33d8aeb#n236\n\nIs that not true anymore?","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c9910f4634ef1deecfde77e39b7a22cb823751f2","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                return data"},{"line_number":609,"context_line":"            if _latest_soft_match(min_version, data[\u0027version\u0027]):"},{"line_number":610,"context_line":"                return data"},{"line_number":611,"context_line":"            if min_version and max_version and _version_between("},{"line_number":612,"context_line":"                    min_version, max_version, data[\u0027version\u0027]):"},{"line_number":613,"context_line":"                return data"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_f539f93e","line":611,"in_reply_to":"ff346bd7_e7439dae","updated":"2017-07-25 15:09:47.000000000","message":"Well, not exactly.  http://git.openstack.org/cgit/openstack/keystoneauth/tree/keystoneauth1/discover.py?id\u003d218adc333efe0cea8f01ee11666e53f4c33d8aeb#n226 (ten lines earlier) we return False if *both* min and max are None.  I felt that was contradictory to the behavior in the rest of the method, where None means \"no bound\", which is why I removed that part; but that made this piece necessary.","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"a05bcb321034cef4ca1384cd1d50744f89782135","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                return data"},{"line_number":609,"context_line":"            if _latest_soft_match(min_version, data[\u0027version\u0027]):"},{"line_number":610,"context_line":"                return data"},{"line_number":611,"context_line":"            if min_version and max_version and _version_between("},{"line_number":612,"context_line":"                    min_version, max_version, data[\u0027version\u0027]):"},{"line_number":613,"context_line":"                return data"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_f53e8ed8","line":611,"in_reply_to":"ff346bd7_f539f93e","updated":"2017-07-25 20:41:42.000000000","message":"I see - but now it seems like whether you put it inside or outside the method is arbitrary. You could reduce some code by putting it back in the method.","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"df1a544219c98c20a712c9da5e826b86c575f8ab","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                return data"},{"line_number":609,"context_line":"            if _latest_soft_match(min_version, data[\u0027version\u0027]):"},{"line_number":610,"context_line":"                return data"},{"line_number":611,"context_line":"            if min_version and max_version and _version_between("},{"line_number":612,"context_line":"                    min_version, max_version, data[\u0027version\u0027]):"},{"line_number":613,"context_line":"                return data"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_5b90eb43","line":611,"in_reply_to":"ff346bd7_f53e8ed8","updated":"2017-07-25 21:28:28.000000000","message":"You\u0027re right about reducing the code - but the decision was not arbitrary.  I felt that _version_between ought to treat min/max of None consistently, and I felt it strongly enough to be willing to have these extra checks (here and L965) duplicated outside of that method.  I could not think of a way to describe the min_version and max_version parameters to _version_between in such a way that it actually made sense for (min\u003dNone, max\u003dNone)\u003d\u003eFalse along with the other behaviors.","commit_id":"e6e7fcfef394f0d93da55fb9059929011ed3fed3"}]}