)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"6bfede0502fe45168b0bc215c5010fc9878066da","unresolved":false,"context_lines":[{"line_number":25,"context_line":"elements we have chosen.  Several elements need to walk the element"},{"line_number":26,"context_line":"list to pickup files, and we do *not* want them to re-implement this"},{"line_number":27,"context_line":"override logic.  A follow-on change will expose the list of elements"},{"line_number":28,"context_line":"and their path in a separate variable that can be parsed by elements."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Change-Id: I2a29861c67de2d25c595cb35d850e92807d26ac6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3aaa91ec_6404b517","line":28,"updated":"2016-06-30 15:19:10.000000000","message":"This is interesting, and I think I like it, but it is an interface break. Currently we support the (albeit odd) behavior of merging scripts within two elements that have the same name but different script names. So, if we go through with this, we are going to have to remember to major version bump and then deal with the potential consequences of that (hopefully we don\u0027t have to backport any bugfixes because then we\u0027ll need to make stable branches).","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"409c4a4053ba8b0feece4b8213529bcb249c3ab6","unresolved":false,"context_lines":[{"line_number":25,"context_line":"elements we have chosen.  Several elements need to walk the element"},{"line_number":26,"context_line":"list to pickup files, and we do *not* want them to re-implement this"},{"line_number":27,"context_line":"override logic.  A follow-on change will expose the list of elements"},{"line_number":28,"context_line":"and their path in a separate variable that can be parsed by elements."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Change-Id: I2a29861c67de2d25c595cb35d850e92807d26ac6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3aaa91ec_f8c005e2","line":28,"in_reply_to":"3aaa91ec_6404b517","updated":"2016-07-01 00:17:56.000000000","message":"So I don\u0027t believe this actually breaks the interface for those elements that did it like this.\n\nIMAGE_ELEMENT (the list of elements) is still exported in the same way, and if an element chooses to walk ELEMENTS_PATH by itself and find each element in that list according to its own rules, it can.  \n\nI0a64b45e9f2cfa28e84b2859d76b065a6c4590f0 is where i actually convert svc-map \u0026 pkg-map to use these explicit exports, so i guess the point stands there.  maybe we should discuss if we want to support the old behaviour in that.\n\nthis change is really more about standardising on a documented interface for elements to find other elements, for new elements to use going forward.\n\nre-reading this, though, i could make that clearer","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"b13629cf18dcdb4d6926d37a0929960368e1df54","unresolved":false,"context_lines":[{"line_number":25,"context_line":"elements we have chosen.  Several elements need to walk the element"},{"line_number":26,"context_line":"list to pickup files, and we do *not* want them to re-implement this"},{"line_number":27,"context_line":"override logic.  A follow-on change will expose the list of elements"},{"line_number":28,"context_line":"and their path in a separate variable that can be parsed by elements."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Change-Id: I2a29861c67de2d25c595cb35d850e92807d26ac6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1aa78d24_2670c30a","line":28,"in_reply_to":"3aaa91ec_f8c005e2","updated":"2016-07-12 22:51:35.000000000","message":"The issue is that I can have an element elsewhere in my ELEMENTS_PATH called ubuntu that has an install.d/50-something-special and build an image using ubuntu (so there will be both elements in the search path). The result right now would be that the in-tree ubuntu would be used as well as the one with 50-something-special. After this change only the scripts in one of the elements would be used.","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"}],"diskimage_builder/element_dependencies.py":[{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"6bfede0502fe45168b0bc215c5010fc9878066da","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    :param user_elements: iterable enumerating the elements a user requested"},{"line_number":82,"context_line":"    :param all_elements: Element object dictionary from find_all_elements"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :return: a set containing user_elements and all dependent elements"},{"line_number":85,"context_line":"             including any transitive dependencies."},{"line_number":86,"context_line":"    \"\"\""},{"line_number":87,"context_line":"    final_elements \u003d set(user_elements)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_5ef15a94","line":84,"updated":"2016-06-30 15:19:10.000000000","message":"Can we make this more specific of the type - this description now reads like it returns an element object but it really returns a string IIUC.\n\nI actually think this might be better if it returned element objects. The next logical step is to make use of the exact path of elements outside of this method (your depending patch does this, and I also would like this for my remote elements patch) and that will make it more simple to do this (no need to store a dict and re-map to the original element).","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"6f5a4c680b5f7c090d8d376c9adc1bf7cce95282","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    :param user_elements: iterable enumerating the elements a user requested"},{"line_number":82,"context_line":"    :param all_elements: Element object dictionary from find_all_elements"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :return: a set containing user_elements and all dependent elements"},{"line_number":85,"context_line":"             including any transitive dependencies."},{"line_number":86,"context_line":"    \"\"\""},{"line_number":87,"context_line":"    final_elements \u003d set(user_elements)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_5f8f9674","line":84,"in_reply_to":"1aa78d24_5d76bea4","updated":"2016-07-12 20:29:53.000000000","message":"This should not be an API point.  The API is the list of elements this produces -- exposed via the YAML environment variable and the bash function.  There should be no need to run this twice, and essentially no element or tool should care how we get the element list -- just that it is there","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"c9dfc8e5bfed500c258a8e017959af27202170eb","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    :param user_elements: iterable enumerating the elements a user requested"},{"line_number":82,"context_line":"    :param all_elements: Element object dictionary from find_all_elements"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :return: a set containing user_elements and all dependent elements"},{"line_number":85,"context_line":"             including any transitive dependencies."},{"line_number":86,"context_line":"    \"\"\""},{"line_number":87,"context_line":"    final_elements \u003d set(user_elements)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_86e257d3","line":84,"in_reply_to":"1aa78d24_5f8f9674","updated":"2016-07-12 22:48:22.000000000","message":"ok, I think there is a larger issue here which we don\u0027t need to solve in this patch that we don\u0027t consider our python api public. We really should be prefixing everything we don\u0027t consider public with a _. The larger change is probably to make this whole module _element_dependencies.py and break out the public bits to a cmds.py or similar, that can happen in another change though.\n\n\nI\u0027m fine with not changing this for, though.","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"409c4a4053ba8b0feece4b8213529bcb249c3ab6","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    :param user_elements: iterable enumerating the elements a user requested"},{"line_number":82,"context_line":"    :param all_elements: Element object dictionary from find_all_elements"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :return: a set containing user_elements and all dependent elements"},{"line_number":85,"context_line":"             including any transitive dependencies."},{"line_number":86,"context_line":"    \"\"\""},{"line_number":87,"context_line":"    final_elements \u003d set(user_elements)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_d8ef09e7","line":84,"in_reply_to":"3aaa91ec_5ef15a94","updated":"2016-07-01 00:17:56.000000000","message":"The description I can fix\n\nI don\u0027t think that expand_dependencies should be considered an interface, or called more than once.  Each time it walks the tree and does the whole calculation again which -- although very minor i admit -- introduces the possibility things change; but in general is just unnecessary.\n\nI\u0027d like to see elements settle on the interface that they either decode IMAGE_ELEMENTS_YAML (python elements) or use the bash array of the same (i.e. Ibbd07d082ec827441def2d3f6240df3efdc6eae3).  These two variables are the API.\n\ni.e. there\u0027s not much point returning Element objects, because these objects are just an intermediate state before they are exported to our canonical reference variables.","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"8fdcdac3541adc86fb6363e4e7860379b9274bb5","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    :param user_elements: iterable enumerating the elements a user requested"},{"line_number":82,"context_line":"    :param all_elements: Element object dictionary from find_all_elements"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :return: a set containing user_elements and all dependent elements"},{"line_number":85,"context_line":"             including any transitive dependencies."},{"line_number":86,"context_line":"    \"\"\""},{"line_number":87,"context_line":"    final_elements \u003d set(user_elements)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_5d76bea4","line":84,"in_reply_to":"3aaa91ec_8c7423cf","updated":"2016-07-12 17:30:52.000000000","message":"If we do this later on it will need to be as a separate method that maps over the returned value of this since it is a public api.\n\nFor the test cases - I think implement the set operations on Element (__eq__, __ne__, __hash__) and assertEqual(set(\u0027...\u0027), set(returned_value))?","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"da7618090671b6f8b12e04f009dd16777d8d1b12","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    :param user_elements: iterable enumerating the elements a user requested"},{"line_number":82,"context_line":"    :param all_elements: Element object dictionary from find_all_elements"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :return: a set containing user_elements and all dependent elements"},{"line_number":85,"context_line":"             including any transitive dependencies."},{"line_number":86,"context_line":"    \"\"\""},{"line_number":87,"context_line":"    final_elements \u003d set(user_elements)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_8c7423cf","line":84,"in_reply_to":"3aaa91ec_d8ef09e7","updated":"2016-07-01 02:48:11.000000000","message":"Ahh, I also see what you mean that it could return a list of Element() objects.\n\nThat means refactoring all the test-cases, because they\u0027re doing \"assertEqual( set(\u0027...\u0027), returned_value)\"\n\nso they all have to convert this list of Element objects back to strings etc.  i\u0027m not totally opposed to this, but there does not seem to be any benefit to this considerable disruption.  if we want to refactor it like that, should be a follow-on change to avoid confusing the issue","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":10035,"name":"greghaynes","email":"greg@greghaynes.net","username":"greghaynes"},"change_message_id":"6bfede0502fe45168b0bc215c5010fc9878066da","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    Walk ELEMENTS_PATH and find all elements.  Make an Element object"},{"line_number":134,"context_line":"    for each element we wish to consider.  Note we process overrides"},{"line_number":135,"context_line":"    such that elements specified earlier in the ELEMENTS_PATH override"},{"line_number":136,"context_line":"    those seen later."},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    :param paths: A list of paths to find elements in.  If None will"},{"line_number":139,"context_line":"                  use ELEMENTS_PATH"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_3fc3d658","line":136,"updated":"2016-06-30 15:19:10.000000000","message":"Can we get a test in for the override behavior?","commit_id":"7e95e8bc9e2599f3fa1540068ba393db1a6962fe"},{"author":{"_account_id":12459,"name":"Stephanie Miller","email":"stephane@alum.mit.edu","username":"cinerama"},"change_message_id":"f3e9289211a8259d02d290abc19d871422bef97b","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"def find_all_elements(paths\u003dNone):"},{"line_number":132,"context_line":"    \"\"\"Build a dictionary Element() objects"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    Walk ELEMENTS_PATH and find all elements.  Make an Element object"},{"line_number":135,"context_line":"    for each element we wish to consider.  Note we process overrides"}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_b3ef91af","line":132,"range":{"start_line":132,"start_character":25,"end_line":132,"end_character":26},"updated":"2016-07-15 22:36:45.000000000","message":"nit: of","commit_id":"983aee6a4153b90467d8858ff5db75b3390d17ad"}]}