)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Andreas Florath \u003candreas@florath.net\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-04-11 19:16:40 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor: Cleanup block device config"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch refactors / cleans up the configuration handling"},{"line_number":10,"context_line":"of the block device layer; especially it introduces a tree-like"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7ffa3b31_eb38cbee","line":7,"updated":"2017-04-20 06:19:13.000000000","message":"It\u0027s a minor quibble, but I don\u0027t think this is a refactor.  I would expect to see a negative diffstat if we\u0027re cleaning things up.  this is not that :)","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"}],"diskimage_builder/block_device/__init__.py":[{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"fbabf0db50ec4dc8b03f72e802789d1984f34fb6","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    args \u003d parser.parse_args()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    logger.info(\"phase [%s]\" % args.phase)"},{"line_number":55,"context_line":"    logger.info(\"params [%s]\" % args.params)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    bd \u003d BlockDevice(args)"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_77a94022","line":55,"range":{"start_line":55,"start_character":4,"end_line":55,"end_character":10},"updated":"2017-04-10 12:55:29.000000000","message":"you shall login symbol as well if it exists, if you login phase and paraams?","commit_id":"8f987948333c677eaa4103f5687c8d18d7a37107"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"745a8631cf3a2afd0c900a1a662971092f11fac6","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    args \u003d parser.parse_args()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    logger.info(\"phase [%s]\" % args.phase)"},{"line_number":55,"context_line":"    logger.info(\"params [%s]\" % args.params)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    bd \u003d BlockDevice(args)"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_5e2e22fb","line":55,"range":{"start_line":55,"start_character":4,"end_line":55,"end_character":10},"in_reply_to":"bff0334d_77a94022","updated":"2017-04-11 19:17:11.000000000","message":"Done","commit_id":"8f987948333c677eaa4103f5687c8d18d7a37107"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                        help\u003d\"phase to execute\")"},{"line_number":48,"context_line":"    parser.add_argument(\u0027--params\u0027, required\u003dTrue,"},{"line_number":49,"context_line":"                        help\u003d\"parameters for block device handling\")"},{"line_number":50,"context_line":"    parser.add_argument(\u0027--symbol\u0027, required\u003dFalse,"},{"line_number":51,"context_line":"                        help\u003d\"symbol to query for getval\")"},{"line_number":52,"context_line":"    args \u003d parser.parse_args()"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_96116e55","line":50,"updated":"2017-04-20 06:19:13.000000000","message":"is this really related to the tree config?","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"}],"diskimage_builder/block_device/blockdevice.py":[{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"fbabf0db50ec4dc8b03f72e802789d1984f34fb6","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    In a script this should be called in the following way:"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    dib-block-device --phase\u003dinit ..."},{"line_number":74,"context_line":"    # From that point the database can be quieried, like"},{"line_number":75,"context_line":"    ROOT_LABEL\u003d$(dib-block-device --phase\u003dgetval --symbol\u003droot-label ...)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    dib-block-device --phase\u003dcreate ..."}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_3765f81e","line":74,"range":{"start_line":74,"start_character":42,"end_line":74,"end_character":50},"updated":"2017-04-10 12:55:29.000000000","message":"typo, queried... also, this cannot be accessed outside the chroot, is there a way you can make it accessible, so we can consume from elements like the selinux fixing?","commit_id":"8f987948333c677eaa4103f5687c8d18d7a37107"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"745a8631cf3a2afd0c900a1a662971092f11fac6","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    In a script this should be called in the following way:"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    dib-block-device --phase\u003dinit ..."},{"line_number":74,"context_line":"    # From that point the database can be quieried, like"},{"line_number":75,"context_line":"    ROOT_LABEL\u003d$(dib-block-device --phase\u003dgetval --symbol\u003droot-label ...)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    dib-block-device --phase\u003dcreate ..."}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_fe84d6d4","line":74,"range":{"start_line":74,"start_character":42,"end_line":74,"end_character":50},"in_reply_to":"bff0334d_3765f81e","updated":"2017-04-11 19:17:11.000000000","message":"Typo fixed.\n\nRest of your remark: I know about this - but I really want to do things step by step (this patch is already HUGE).","commit_id":"8f987948333c677eaa4103f5687c8d18d7a37107"}],"diskimage_builder/block_device/level0/localloop.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    This class handles local loop devices that can be used"},{"line_number":33,"context_line":"    for VM image installation."},{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":"    tree_config \u003d TreeConfig(\"local_loop\")"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def __init__(self, config, default_config):"},{"line_number":38,"context_line":"        logger.debug(\"Creating LocalLoop object; config [%s] \""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_760022ac","line":35,"updated":"2017-04-20 06:19:13.000000000","message":"This seems to be a required property, but is not defined in the ABC at all?\n\nIf this is common and always required, it seems it would be better for NodePluginBase to define a common __init__() and for us to make sure we do \n\n  super(NodePluginBase, self).__init__(tree_config\u003d\"local_loop\")\n\nand have *that* set self.tree_config?  or maybe pass in the object.","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            self.size \u003d parse_abs_size_spec(config[\u0027size\u0027])"},{"line_number":42,"context_line":"            logger.debug(\"Image size [%s]\" % self.size)"},{"line_number":43,"context_line":"        else:"},{"line_number":44,"context_line":"            self.size \u003d parse_abs_size_spec(default_config[\u0027image-size\u0027])"},{"line_number":45,"context_line":"            logger.debug(\"Using default image size [%s]\" % self.size)"},{"line_number":46,"context_line":"        if \u0027directory\u0027 in config:"},{"line_number":47,"context_line":"            self.image_dir \u003d config[\u0027directory\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_16355e8f","line":44,"updated":"2017-04-20 06:19:13.000000000","message":"?  why in this change","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        if \u0027directory\u0027 in config:"},{"line_number":47,"context_line":"            self.image_dir \u003d config[\u0027directory\u0027]"},{"line_number":48,"context_line":"        else:"},{"line_number":49,"context_line":"            self.image_dir \u003d default_config[\u0027image-dir\u0027]"},{"line_number":50,"context_line":"        self.name \u003d config[\u0027name\u0027]"},{"line_number":51,"context_line":"        Digraph.Node.__init__(self, self.name)"},{"line_number":52,"context_line":"        self.filename \u003d os.path.join(self.image_dir, self.name + \".raw\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_f625525d","line":49,"updated":"2017-04-20 06:19:13.000000000","message":"? why in this change?","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        block_device \u003d self._loopdev_attach(self.filename)"},{"line_number":114,"context_line":"        rollback.append(lambda: self._loopdev_detach(block_device))"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        if \u0027blockdev\u0027 not in result:"},{"line_number":117,"context_line":"            result[\u0027blockdev\u0027] \u003d {}"},{"line_number":118,"context_line":"        result[\u0027blockdev\u0027][self.name] \u003d {\"device\": block_device,"},{"line_number":119,"context_line":"                                         \"image\": self.filename}"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_b6234a56","line":116,"updated":"2017-04-20 06:19:13.000000000","message":"?  where did \"result\" change?  is it necessary here?  can you comment this as to when this will trigger?","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"}],"diskimage_builder/block_device/level1/partitioning.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":31,"context_line":"class PartitionTreeConfig(object):"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    @staticmethod"},{"line_number":34,"context_line":"    def config_tree_to_digraph(config_key, config_value, pconfig, dconfig,"},{"line_number":35,"context_line":"                               base_name, plugin_manager):"},{"line_number":36,"context_line":"        logger.debug(\"called [%s] [%s] [%s]\""},{"line_number":37,"context_line":"                     % (config_key, config_value, base_name))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_eb968b3e","line":34,"updated":"2017-04-20 06:19:13.000000000","message":"This is a *6* argument function that does not have a single comment.  at a minimum, this needs a standard function header.  look at element_dependencies and the :param: type headers for what i mean. \n\nA class with a single static function does not need to be a class.  Just make it a function; we\u0027re adding levels of nesting that make things too hard to follow.","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                    plugin_manager[k].plugin \\"},{"line_number":48,"context_line":"                                     .tree_config.config_tree_to_digraph("},{"line_number":49,"context_line":"                                         k, v, dconfig, name, plugin_manager)"},{"line_number":50,"context_line":"            pconfig.append(nconfig)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        logger.debug(\"finished [%s] [%s]\" % (nconfig, dconfig))"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_2bba33c0","line":50,"updated":"2017-04-20 06:19:13.000000000","message":"this whole bit needs a higher level comment.","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        self._exec_sudo([\"kpartx\", \"-avs\", device_path])"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    def create(self, result, rollback):"},{"line_number":269,"context_line":"        image_path \u003d result[\u0027blockdev\u0027][self.base][\u0027image\u0027]"},{"line_number":270,"context_line":"        device_path \u003d result[\u0027blockdev\u0027][self.base][\u0027device\u0027]"},{"line_number":271,"context_line":"        logger.info(\"Creating partition on [%s] [%s]\" %"},{"line_number":272,"context_line":"                    (self.base, image_path))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_0be417e0","line":269,"updated":"2017-04-20 06:19:13.000000000","message":"why has this changed?","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"}],"diskimage_builder/block_device/tree_config.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    def __init__(self, type_string):"},{"line_number":37,"context_line":"        self.type_string \u003d type_string"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def config_tree_to_digraph(self, config_key, config_value, dconfig,"},{"line_number":40,"context_line":"                               default_base_name, plugin_manager):"},{"line_number":41,"context_line":"        logger.debug(\"called [%s] [%s] [%s]\""},{"line_number":42,"context_line":"                     % (config_key, config_value, default_base_name))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_0b6ad7ec","line":39,"updated":"2017-04-20 06:19:13.000000000","message":"There seems to be this thing happening around the place where you are passing arguments in to be modified.  dconfig seems to be an example here\n\nI\u0027m not sure if this is supposed to be emulating a C-type idiom like stat(3) where you pass a pointer to some defined space to put the result in.  That has it\u0027s place, but I strongly think that\u0027s an anti-pattern in a gc type language as Python; more importantly for other people reading this, it\u0027s also something that is rarely done in openstack code.  \n\nit\u0027s way too easy to get confused as to what\u0027s input and output and rebind it, and it ties implementation and calling too closely together.\n\nFor example here, if we were to somehow be fiddling and change this to dconfig\u003d{} we\u0027d reset our dconfig to a new object, and the callee would just get their old object.  it\u0027s all totally opaque.\n\nthe other thing is, what if the callee wants dconfig to be something else that doesn\u0027t have an \".append()\".  why does this care?  there\u0027s nothing at all when you read this telling us what dconfig has to be, or checks on it\u0027s type, etc.\n\nIn this case it seems it should just return a new list, and the callee can append to the config as it sees fit.  I think the same thing happens when calling the plugin_manager[k].plugin.tree_config.config_tree_to_digraph().\n\ntl;dr -- please make function return something, and consider arguments immutable unless there\u0027s exceptional reasons, and if there are, make sure they\u0027re well documented.","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"}],"diskimage_builder/lib/common-functions":[{"author":{"_account_id":6133,"name":"yolanda.robla","email":"yroblamo@redhat.com","username":"yolanda.robla"},"change_message_id":"fbabf0db50ec4dc8b03f72e802789d1984f34fb6","unresolved":false,"context_lines":[{"line_number":398,"context_line":"- local_loop:"},{"line_number":399,"context_line":"    name: image0"},{"line_number":400,"context_line":"    mkfs:"},{"line_number":401,"context_line":"      name: mkfs_root"},{"line_number":402,"context_line":"      mount:"},{"line_number":403,"context_line":"        mount_point: /"},{"line_number":404,"context_line":"        fstab:"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"bff0334d_d7a12c48","line":401,"range":{"start_line":401,"start_character":6,"end_line":401,"end_character":10},"updated":"2017-04-10 12:55:29.000000000","message":"should you specify the ext4 file type?","commit_id":"8f987948333c677eaa4103f5687c8d18d7a37107"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"745a8631cf3a2afd0c900a1a662971092f11fac6","unresolved":false,"context_lines":[{"line_number":398,"context_line":"- local_loop:"},{"line_number":399,"context_line":"    name: image0"},{"line_number":400,"context_line":"    mkfs:"},{"line_number":401,"context_line":"      name: mkfs_root"},{"line_number":402,"context_line":"      mount:"},{"line_number":403,"context_line":"        mount_point: /"},{"line_number":404,"context_line":"        fstab:"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"bff0334d_9efcaa3e","line":401,"range":{"start_line":401,"start_character":6,"end_line":401,"end_character":10},"in_reply_to":"bff0334d_d7a12c48","updated":"2017-04-11 19:17:11.000000000","message":"If we use \u0027ext4\u0027 here there is no way to overwrite this setting via $FS_TYPE - which means dropping support for this.\nWhat do you think?","commit_id":"8f987948333c677eaa4103f5687c8d18d7a37107"}],"diskimage_builder/lib/disk-image-create":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5ff0c78b57f3465a5dda1ae39bff7592d9285165","unresolved":false,"context_lines":[{"line_number":277,"context_line":""},{"line_number":278,"context_line":"mk_build_dir"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"# Create the YAML file with the final and raw configuration for"},{"line_number":281,"context_line":"# the block device layer."},{"line_number":282,"context_line":"mkdir -p ${TMP_BUILD_DIR}/block-device"},{"line_number":283,"context_line":"BLOCK_DEVICE_CONFIG_YAML\u003d${TMP_BUILD_DIR}/block-device/config.yaml"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"7ffa3b31_4b545f21","line":280,"updated":"2017-04-20 06:19:13.000000000","message":"does any of this have to be in this change?  can we not deal tree configuration, and then consider this next.","commit_id":"f9dced08bdcd3783d1ed371fc5d755f516bae8ee"}]}