)]}'
{"diskimage_builder/block_device/blockdevice.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"f2bd4ecd5bcdf6b053666ece4096ace0536f994a","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            json.dump(state, fd)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def create(self, result, rollback):"},{"line_number":161,"context_line":"        dg, call_order \u003d create_graph(self.config, self.params, result)"},{"line_number":162,"context_line":"        for node in call_order:"},{"line_number":163,"context_line":"            node.create(rollback)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_169f2d51","line":161,"updated":"2017-05-29 02:50:38.000000000","message":"remove this.  it\u0027s only called once, named the same as the node api function but not quite the same, and calls \"state\" by a different name unnecessarily.  better just in cmd_create()","commit_id":"8c513f15fed323d690266347835b5215d707808a"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"f2bd4ecd5bcdf6b053666ece4096ace0536f994a","unresolved":false,"context_lines":[{"line_number":292,"context_line":"        logger.info(\"create() called\")"},{"line_number":293,"context_line":"        logger.debug(\"Using config [%s]\", self.config)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        self.state \u003d {}"},{"line_number":296,"context_line":"        rollback \u003d []"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_d6a89578","line":295,"updated":"2017-05-29 02:50:38.000000000","message":"why does this do this after it was set in init?\n\nedit ... oh, because it seems on init self.state \u003d _load_json(state_file_name), which sets it to \"None\" if that doesn\u0027t exist; it doesn\u0027t create a new dictionary.\n\nMove this up to where it is initalized the first time; at least something like?\n\n self.state \u003d _load_json(state_file)\n if not self.state:\n   # there was no json file, so we must be \n   # starting fresh\n\n?  if state is !None here, that seems to indicate something odd going on too.\n\nfurther edit ... oh, i see cmd_umount checks for a None value, menaing piror comment doesn\u0027t work.  but then cleanup() and delete() don\u0027t ... they seem to assume it is there, but then *might* pass through \"None\" as a value for the dictionary when creating the graph too?\n\n---\n\nthere is insufficient detail here for a reader to determine what the intent.  it needs comments and appropriate checks","commit_id":"8c513f15fed323d690266347835b5215d707808a"}],"diskimage_builder/block_device/level0/localloop.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"f2bd4ecd5bcdf6b053666ece4096ace0536f994a","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        return"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def umount(self):"},{"line_number":124,"context_line":"        self._loopdev_detach(self.get_state()[\u0027blockdev\u0027][self.name][\u0027device\u0027])"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def cleanup(self):"},{"line_number":127,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_56e125fe","line":124,"updated":"2017-05-29 02:50:38.000000000","message":"this is definitely not pythonic.  \n\ni think it\u0027s fine to simply have this as a known, fixed attribute.\n\n  self.state[\u0027...\u0027]\n\ngetter/setter is usually considered a bit of a python anti-pattern anyway.  we could remove get_name() for consistency with this ... don\u0027t think we even use it, we have \"object.name\" everywhere.","commit_id":"8c513f15fed323d690266347835b5215d707808a"}],"diskimage_builder/block_device/plugin.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"f2bd4ecd5bcdf6b053666ece4096ace0536f994a","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    order to perform its operations."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    Every node has a unique string ``name``.  This is its key in the"},{"line_number":34,"context_line":"    graph and used for edge relationships.  In addition the ``state``"},{"line_number":35,"context_line":"    object is passed in to the constructor.  That makes it possible"},{"line_number":36,"context_line":"    that even during initializion (e.g. config check) the state"},{"line_number":37,"context_line":"    can be accessed."}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_16472d20","line":34,"updated":"2017-05-29 02:50:38.000000000","message":"I would not write this as if the reader knows what the \"state\" object is.  the :param: documentation is better.  this should say something like\n\n\"``state`` is a reference to a shared global state dictionary.  plugins can reference keys from prior plugin-object instantiations, or place keys for future reference.  Note that since objects are created in order after a topological sort of the configuration graph, object instantiation ordering is not stable.\"\n\nalso, it should probably be explained that this is dumped to disk and passed back in it\u0027s complete state to unmount and cleanup.  while create() calls are only seeing partial state as the iteration over nodes progresses, it seems these two are getting the full state, which is a slightly different concept i guess.","commit_id":"8c513f15fed323d690266347835b5215d707808a"}]}