)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"bccee8e81000ca0862a873365c1ffa96866b4e2b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lee Yarwood \u003clyarwood@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-08 17:07:38 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"fup: Merge machine_type_mappings into get_default_machine_type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As discussed in Id97f4baddcf2caff91599773d9b5de5181b7fdf6"},{"line_number":10,"context_line":"machine_type_mappings only had a single caller and should be merged into"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_19db377c","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":3},"updated":"2019-06-11 08:51:32.000000000","message":"Nit (but necessary): let\u0027s try not to propagate this obscure IRC lingo to formal Git commit messages; use \"fixup\"; it\u0027s far clearer to non-native speakers and those who don\u0027t dwell on IRC.  Clarity is power.","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"c3cf3456245ab4a16bd6fea6440cafbb125a513d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lee Yarwood \u003clyarwood@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-08 17:07:38 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"fup: Merge machine_type_mappings into get_default_machine_type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As discussed in Id97f4baddcf2caff91599773d9b5de5181b7fdf6"},{"line_number":10,"context_line":"machine_type_mappings only had a single caller and should be merged into"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_f028ade2","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":3},"in_reply_to":"9fb8cfa7_08299fe4","updated":"2019-06-11 17:50:07.000000000","message":"\u003e I always thought it meant \"followup\".\n\nWell, if we already have different interpretations, that perfectly illustrates the ambiguity which needs to be eliminated.  My brain always initially interprets it as \"fuck-up\" before realising that that\u0027s probably wrong ;-)  And then it switches to reading it as \"fixup\".  I\u0027m definitely with Kashyap on this kind of obfuscation leading to death by a thousand cuts.\n\nBut isn\u0027t pretty much *every* commit a follow-up anyway?  If we\u0027re going to use this, when should it be used and when not?  And how does it differ from fixups?  Without a clear policy, to me it just seems like adding obfuscated cruft for no benefit.\n\n \u003e The abbreviation is useful for keeping commit headers under 50c.\n \u003e \n \u003e But it could just as easily be left out of the header and included in the body.\n\nEven if it was in the body it\u0027s still not obvious to me what it\u0027s supposed to signify.\n\nI guess maybe we need to take this to the ML, or at least to IRC ...","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"19cccad507eaa9bbbedd72477ac7f20f63b098e9","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lee Yarwood \u003clyarwood@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-08 17:07:38 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"fup: Merge machine_type_mappings into get_default_machine_type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As discussed in Id97f4baddcf2caff91599773d9b5de5181b7fdf6"},{"line_number":10,"context_line":"machine_type_mappings only had a single caller and should be merged into"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_fc53ac69","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":3},"in_reply_to":"9fb8cfa7_08299fe4","updated":"2019-06-12 08:12:22.000000000","message":"[Last one on this here; won\u0027t belabor more.]\n\nThanks for clarifying. Yes, I far prefer your suggestion of expanding it in the commit body.  (And for keeping the commit header under 50 char, it\u0027s an \"editing judo\" that we just need to get good at without taking poor shortcuts ;-))\n\nI will continue pointing it out whenever I see this obfuscation in commit messages.  Not least because it is utterly redundant, as Adam points out; and not make people guess if it is a follow-up, fix-up, or a ****-up.","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d8773551422c873ca8c4f3edc7e06173d02ef10c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lee Yarwood \u003clyarwood@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-08 17:07:38 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"fup: Merge machine_type_mappings into get_default_machine_type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As discussed in Id97f4baddcf2caff91599773d9b5de5181b7fdf6"},{"line_number":10,"context_line":"machine_type_mappings only had a single caller and should be merged into"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_08299fe4","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":3},"in_reply_to":"9fb8cfa7_19db377c","updated":"2019-06-11 13:42:30.000000000","message":"I always thought it meant \"followup\".\n\nThe abbreviation is useful for keeping commit headers under 50c.\n\nBut it could just as easily be left out of the header and included in the body.","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7aa1d007d79d063452dc1c48d2fb92fbc650ca4d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lee Yarwood \u003clyarwood@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-08 17:07:38 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"fup: Merge machine_type_mappings into get_default_machine_type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As discussed in Id97f4baddcf2caff91599773d9b5de5181b7fdf6"},{"line_number":10,"context_line":"machine_type_mappings only had a single caller and should be merged into"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_50d3b9d5","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":3},"in_reply_to":"9fb8cfa7_f028ade2","updated":"2019-06-11 17:53:29.000000000","message":"\u003e \u003e But it could just as easily be left out of the header and\n \u003e included in the body.\n \u003e \n \u003e Even if it was in the body it\u0027s still not obvious to me what it\u0027s\n \u003e supposed to signify.\n\nI meant spelling it out in the body (followup or followon or fixup or any hyphenated variant thereof).\n \n \u003e I guess maybe we need to take this to the ML, or at least to IRC\n \u003e ...\n\nPlease no. We have far more important things to worry about.","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"66444ead3dc27e153e17e0cf0c75aa8827f7bac8","unresolved":false,"context_lines":[{"line_number":7,"context_line":"fup: Merge machine_type_mappings into get_default_machine_type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As discussed in Id97f4baddcf2caff91599773d9b5de5181b7fdf6"},{"line_number":10,"context_line":"machine_type_mappings only had a single caller and should be merged into"},{"line_number":11,"context_line":"get_default_machine_type."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I29c7a96ca858e8941744b1a931863b908522fc4a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fb8cfa7_eb3c40c9","line":11,"range":{"start_line":10,"start_character":22,"end_line":11,"end_character":25},"updated":"2019-06-11 18:14:16.000000000","message":"Nit (with apologies in advance for getting preachy): this risks being interpreted as implying that \"methods/functions with a single caller should be inlined\" is a general rule to be followed, and IMHO that\u0027s a dangerous precedent which should not be set.  There are many occasions on which it\u0027s justified to keep methods/functions separate even when they only have a single caller.  For example, it helps follow the Single Responsibility Principle and avoid code pyramids of doom:\n\nhttps://itnext.io/pyramid-of-doom-the-signs-and-symptoms-of-a-common-anti-pattern-c716838e1819\n\nIt also makes unit testing easier.\n\nHowever this is one of my favourite soapboxes which I polluted https://review.opendev.org/#/c/639091/ with, so I\u0027ll try to resist doing too much of the same here.\n\nSuffice to say, I\u0027d prefer this commit message to list a more compelling motivation for the change (which I\u0027m slightly struggling to see at the moment), but this is already W+1 so a -1 won\u0027t achieve much at this point.","commit_id":"4c65467a4e58e35b69195efef723c5f70c1dd61e"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a743c6fe9be026e1b030ed56bd973b4010d862e9","unresolved":false,"context_lines":[{"line_number":557,"context_line":"def get_default_machine_type(arch):"},{"line_number":558,"context_line":"    # NOTE(lyarwood): Values defined in [libvirt]/hw_machine_type take"},{"line_number":559,"context_line":"    # precedence here if available for the provided arch."},{"line_number":560,"context_line":"    machine_types \u003d {}"},{"line_number":561,"context_line":"    for mapping in CONF.libvirt.hw_machine_type or {}:"},{"line_number":562,"context_line":"        host_arch, _, machine_type \u003d mapping.partition(\u0027\u003d\u0027)"},{"line_number":563,"context_line":"        if machine_type \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_358e65d0","line":560,"range":{"start_line":560,"start_character":0,"end_line":560,"end_character":22},"updated":"2019-06-07 21:25:49.000000000","message":"If this is the only usage, and you\u0027re not going to cache the machine_types mapping, why not take it a step further.\n\nDelete this line...","commit_id":"8db4bde3f21512235fe11d000fb79258cd105e4a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a743c6fe9be026e1b030ed56bd973b4010d862e9","unresolved":false,"context_lines":[{"line_number":563,"context_line":"        if machine_type \u003d\u003d \u0027\u0027:"},{"line_number":564,"context_line":"            LOG.warning(\"Invalid hw_machine_type config value %s\", mapping)"},{"line_number":565,"context_line":"        else:"},{"line_number":566,"context_line":"            machine_types[host_arch] \u003d machine_type"},{"line_number":567,"context_line":"    machine_type \u003d machine_types.get(arch)"},{"line_number":568,"context_line":"    if machine_type:"},{"line_number":569,"context_line":"        return machine_type"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_d5bee963","line":566,"range":{"start_line":566,"start_character":12,"end_line":566,"end_character":51},"updated":"2019-06-07 21:25:49.000000000","message":"...and replace this one with\n\n if host_arch \u003d\u003d arch:\n     return machine_type\n\n...","commit_id":"8db4bde3f21512235fe11d000fb79258cd105e4a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a743c6fe9be026e1b030ed56bd973b4010d862e9","unresolved":false,"context_lines":[{"line_number":564,"context_line":"            LOG.warning(\"Invalid hw_machine_type config value %s\", mapping)"},{"line_number":565,"context_line":"        else:"},{"line_number":566,"context_line":"            machine_types[host_arch] \u003d machine_type"},{"line_number":567,"context_line":"    machine_type \u003d machine_types.get(arch)"},{"line_number":568,"context_line":"    if machine_type:"},{"line_number":569,"context_line":"        return machine_type"},{"line_number":570,"context_line":"    # NOTE(kchamart): For ARMv7 and AArch64, use the \u0027virt\u0027 board as the"},{"line_number":571,"context_line":"    # default machine type.  It is the recommended board, which is designed"},{"line_number":572,"context_line":"    # to be used with virtual machines.  The \u0027virt\u0027 board is more flexible,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_b5bb3570","line":569,"range":{"start_line":567,"start_character":0,"end_line":569,"end_character":27},"updated":"2019-06-07 21:25:49.000000000","message":"...and then you don\u0027t need this...","commit_id":"8db4bde3f21512235fe11d000fb79258cd105e4a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a743c6fe9be026e1b030ed56bd973b4010d862e9","unresolved":false,"context_lines":[{"line_number":573,"context_line":"    # supports PCI, \u0027virtio\u0027, has decent RAM limits, etc."},{"line_number":574,"context_line":"    if arch in (obj_fields.Architecture.ARMV7,"},{"line_number":575,"context_line":"                obj_fields.Architecture.AARCH64):"},{"line_number":576,"context_line":"        machine_type \u003d \"virt\""},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    if arch in (obj_fields.Architecture.S390,"},{"line_number":579,"context_line":"                obj_fields.Architecture.S390X):"},{"line_number":580,"context_line":"        machine_type \u003d \"s390-ccw-virtio\""},{"line_number":581,"context_line":"    return machine_type"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":""},{"line_number":584,"context_line":"def mdev_name2uuid(mdev_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_55b2f995","line":581,"range":{"start_line":576,"start_character":0,"end_line":581,"end_character":23},"updated":"2019-06-07 21:25:49.000000000","message":"...and you can revert these (or not)","commit_id":"8db4bde3f21512235fe11d000fb79258cd105e4a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0808d1342eea722084fc93ee84edde4917d0a202","unresolved":false,"context_lines":[{"line_number":560,"context_line":"    for mapping in CONF.libvirt.hw_machine_type or {}:"},{"line_number":561,"context_line":"        host_arch, _, machine_type \u003d mapping.partition(\u0027\u003d\u0027)"},{"line_number":562,"context_line":"        if machine_type \u003d\u003d \u0027\u0027:"},{"line_number":563,"context_line":"            LOG.warning(\"Invalid hw_machine_type config value %s\", mapping)"},{"line_number":564,"context_line":"        else:"},{"line_number":565,"context_line":"            if host_arch \u003d\u003d arch:"},{"line_number":566,"context_line":"                return machine_type"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_f941833a","line":563,"range":{"start_line":563,"start_character":0,"end_line":563,"end_character":75},"updated":"2019-06-11 08:46:18.000000000","message":"This changes behavior slightly, in that previously the warning would be issued for all invalid entries whereas now it\u0027ll only warn for the ones we encounter before we find what we want (if any). Not sure if that\u0027s an issue or not? Could this have been considered validation of sorts previously?","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"2fd5eac887ea551c06fcc1293561c0e59f2268ae","unresolved":false,"context_lines":[{"line_number":560,"context_line":"    for mapping in CONF.libvirt.hw_machine_type or {}:"},{"line_number":561,"context_line":"        host_arch, _, machine_type \u003d mapping.partition(\u0027\u003d\u0027)"},{"line_number":562,"context_line":"        if machine_type \u003d\u003d \u0027\u0027:"},{"line_number":563,"context_line":"            LOG.warning(\"Invalid hw_machine_type config value %s\", mapping)"},{"line_number":564,"context_line":"        else:"},{"line_number":565,"context_line":"            if host_arch \u003d\u003d arch:"},{"line_number":566,"context_line":"                return machine_type"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_79ea730f","line":563,"range":{"start_line":563,"start_character":0,"end_line":563,"end_character":75},"in_reply_to":"9fb8cfa7_f941833a","updated":"2019-06-11 08:50:42.000000000","message":"That would only be an issue if we couldn\u0027t find a valid mapping, at which point we would print everything out right? I\u0027m not sure we care enough outside of that to retain the previous behaviour.","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0808d1342eea722084fc93ee84edde4917d0a202","unresolved":false,"context_lines":[{"line_number":561,"context_line":"        host_arch, _, machine_type \u003d mapping.partition(\u0027\u003d\u0027)"},{"line_number":562,"context_line":"        if machine_type \u003d\u003d \u0027\u0027:"},{"line_number":563,"context_line":"            LOG.warning(\"Invalid hw_machine_type config value %s\", mapping)"},{"line_number":564,"context_line":"        else:"},{"line_number":565,"context_line":"            if host_arch \u003d\u003d arch:"},{"line_number":566,"context_line":"                return machine_type"},{"line_number":567,"context_line":"    # NOTE(kchamart): For ARMv7 and AArch64, use the \u0027virt\u0027 board as the"},{"line_number":568,"context_line":"    # default machine type.  It is the recommended board, which is designed"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_d916ff49","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":33},"updated":"2019-06-11 08:46:18.000000000","message":"nit: weird that these aren\u0027t on one elif line","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"2fd5eac887ea551c06fcc1293561c0e59f2268ae","unresolved":false,"context_lines":[{"line_number":561,"context_line":"        host_arch, _, machine_type \u003d mapping.partition(\u0027\u003d\u0027)"},{"line_number":562,"context_line":"        if machine_type \u003d\u003d \u0027\u0027:"},{"line_number":563,"context_line":"            LOG.warning(\"Invalid hw_machine_type config value %s\", mapping)"},{"line_number":564,"context_line":"        else:"},{"line_number":565,"context_line":"            if host_arch \u003d\u003d arch:"},{"line_number":566,"context_line":"                return machine_type"},{"line_number":567,"context_line":"    # NOTE(kchamart): For ARMv7 and AArch64, use the \u0027virt\u0027 board as the"},{"line_number":568,"context_line":"    # default machine type.  It is the recommended board, which is designed"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_99ef271d","line":565,"range":{"start_line":564,"start_character":0,"end_line":565,"end_character":33},"in_reply_to":"9fb8cfa7_d916ff49","updated":"2019-06-11 08:50:42.000000000","message":"Done","commit_id":"55ad648962c9fc1848d6def79e49ed046dcd798f"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"a49d4f34b37e30cb6294748e1b91cef6aa5abd85","unresolved":false,"context_lines":[{"line_number":557,"context_line":"def get_default_machine_type(arch):"},{"line_number":558,"context_line":"    # NOTE(lyarwood): Values defined in [libvirt]/hw_machine_type take"},{"line_number":559,"context_line":"    # precedence here if available for the provided arch."},{"line_number":560,"context_line":"    for mapping in CONF.libvirt.hw_machine_type or {}:"},{"line_number":561,"context_line":"        host_arch, _, machine_type \u003d mapping.partition(\u0027\u003d\u0027)"},{"line_number":562,"context_line":"        if machine_type \u003d\u003d \u0027\u0027:"},{"line_number":563,"context_line":"            LOG.warning(\"Invalid hw_machine_type config value %s\", mapping)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_cdff19a9","line":560,"range":{"start_line":560,"start_character":51,"end_line":560,"end_character":53},"updated":"2019-06-11 12:44:57.000000000","message":"https://review.opendev.org/#/c/663011/9/nova/virt/libvirt/utils.py@571 mentions that this should be [] - seems like a good opportunity to fix it, unless you are planning to do that in a follow-up.","commit_id":"4c65467a4e58e35b69195efef723c5f70c1dd61e"}]}
