)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"09dcd24162c1421b4925a1a07d0fdd5def5ffba3","unresolved":false,"context_lines":[{"line_number":13,"context_line":"the image.  It\u0027s better to just not create the image at all."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Also moves the fstrim inside the conditional since there\u0027s no need"},{"line_number":16,"context_line":"to fstrim a filesystem we\u0027re about to delete either."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I168feb18f0d578b3babbe4784d3ef75e755e1ebd"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3aaa91ec_3be1aeff","line":16,"updated":"2016-06-27 18:45:57.000000000","message":"I am not a fan of \"also\" in commits. These feel like two different concerns, and this change is blocking the more important change, which is to avoid building a useless image. Andreas brings up a good point, that fstrim before umounts is quite different than after, so some analysis of that would be good to have in a second commit. So my recommendation would be to remove the fstrim change from this one, and stack it on top, so we can discuss it separate from the otherwise uncontrovertial change to stop building that image file.","commit_id":"6a486ad9162a12896cc773daf392110d2eb35fd6"}],"bin/disk-image-create":[{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"09dcd24162c1421b4925a1a07d0fdd5def5ffba3","unresolved":false,"context_lines":[{"line_number":455,"context_line":"unmount_image"},{"line_number":456,"context_line":"cleanup_build_dir"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ ironic-agent ]]; then"},{"line_number":459,"context_line":"  # Prep filesystem by discarding all unused space"},{"line_number":460,"context_line":"  fstrim_image"},{"line_number":461,"context_line":"  has_raw_type\u003d"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3aaa91ec_9be6e214","line":458,"updated":"2016-06-27 18:45:57.000000000","message":"I agree! Special casing element names here is wrong. A proper interface with good documentation is 100% the right way to go. However, it was already done, so this is just an optimization. So I don\u0027t think it\u0027s appropriate to block this patch while a new interface is developed.","commit_id":"6a486ad9162a12896cc773daf392110d2eb35fd6"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"5e7fc38f1ea0b618ef9746a9d7c238370a7022f5","unresolved":false,"context_lines":[{"line_number":455,"context_line":"unmount_image"},{"line_number":456,"context_line":"cleanup_build_dir"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ ironic-agent ]]; then"},{"line_number":459,"context_line":"  # Prep filesystem by discarding all unused space"},{"line_number":460,"context_line":"  fstrim_image"},{"line_number":461,"context_line":"  has_raw_type\u003d"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3aaa91ec_4fbfe6a9","line":458,"updated":"2016-06-24 19:26:50.000000000","message":"I\u0027d prefer to have here a check for something like \u0027IMAGE_OUTPUT_NEEDED\u0027 (or something similar) that can be set at some point (e.g. in the vm element).\nExample: for pure docker builds this could also be skipped, and it is getting costly to have all the possible checks done here.","commit_id":"6a486ad9162a12896cc773daf392110d2eb35fd6"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"1934b94568c4a7725fab9628ab4d0452a7ad0999","unresolved":false,"context_lines":[{"line_number":455,"context_line":"unmount_image"},{"line_number":456,"context_line":"cleanup_build_dir"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ ironic-agent ]]; then"},{"line_number":459,"context_line":"  # Prep filesystem by discarding all unused space"},{"line_number":460,"context_line":"  fstrim_image"},{"line_number":461,"context_line":"  has_raw_type\u003d"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3aaa91ec_90fd1871","line":458,"in_reply_to":"3aaa91ec_9be6e214","updated":"2016-06-27 21:12:11.000000000","message":"TBH, I think the interface for this can be pretty simple.  I\u0027ll push my proposal, and if it requires significant discussion then it\u0027ll be easy to split the element-specific version back out.","commit_id":"6a486ad9162a12896cc773daf392110d2eb35fd6"},{"author":{"_account_id":21741,"name":"Andreas Florath","email":"andreas@florath.net","username":"florath"},"change_message_id":"5e7fc38f1ea0b618ef9746a9d7c238370a7022f5","unresolved":false,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ ironic-agent ]]; then"},{"line_number":459,"context_line":"  # Prep filesystem by discarding all unused space"},{"line_number":460,"context_line":"  fstrim_image"},{"line_number":461,"context_line":"  has_raw_type\u003d"},{"line_number":462,"context_line":"  if [ \"$IS_RAMDISK\" \u003d\u003d \"0\" ]; then"},{"line_number":463,"context_line":"    for IMAGE_TYPE in ${IMAGE_TYPES[@]} ; do"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3aaa91ec_ef0e92e3","line":460,"updated":"2016-06-24 19:26:50.000000000","message":"IMHO moving this here does not make sense: it cleans up the /mnt which is unmounted already in line 455.","commit_id":"6a486ad9162a12896cc773daf392110d2eb35fd6"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"1934b94568c4a7725fab9628ab4d0452a7ad0999","unresolved":false,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ ironic-agent ]]; then"},{"line_number":459,"context_line":"  # Prep filesystem by discarding all unused space"},{"line_number":460,"context_line":"  fstrim_image"},{"line_number":461,"context_line":"  has_raw_type\u003d"},{"line_number":462,"context_line":"  if [ \"$IS_RAMDISK\" \u003d\u003d \"0\" ]; then"},{"line_number":463,"context_line":"    for IMAGE_TYPE in ${IMAGE_TYPES[@]} ; do"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3aaa91ec_50d29035","line":460,"in_reply_to":"3aaa91ec_ef0e92e3","updated":"2016-06-27 21:12:11.000000000","message":"Yeah, looks like moving this broke everything.  I\u0027ll put it back.","commit_id":"6a486ad9162a12896cc773daf392110d2eb35fd6"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"b29cd5d8de14bc7901ebb7cc7cc89ead322ab9d1","unresolved":false,"context_lines":[{"line_number":462,"context_line":""},{"line_number":463,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ no-final-image ]]; then"},{"line_number":464,"context_line":"  has_raw_type\u003d"},{"line_number":465,"context_line":"  if [ \"$IS_RAMDISK\" \u003d\u003d \"0\" ]; then"},{"line_number":466,"context_line":"    for IMAGE_TYPE in ${IMAGE_TYPES[@]} ; do"},{"line_number":467,"context_line":"      # We have to do raw last because it is destructive"},{"line_number":468,"context_line":"      if [ \"$IMAGE_TYPE\" \u003d \"raw\" ]; then"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"3aaa91ec_4295693e","line":465,"updated":"2016-06-28 20:16:24.000000000","message":"i\u0027d kind of assumed that\u0027s what IS_RAMDISK is doing here (skipping for ironic-agent)?\n\nso is this check irrelevant?\n\nif not i\u0027d like to see a level of nesting removed and just put this in an ||","commit_id":"42bcf1d4df3892d003acc17e33d33e173610b976"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"7ad49ba425599e76aeea234c224e01763494a6dc","unresolved":false,"context_lines":[{"line_number":462,"context_line":""},{"line_number":463,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ no-final-image ]]; then"},{"line_number":464,"context_line":"  has_raw_type\u003d"},{"line_number":465,"context_line":"  if [ \"$IS_RAMDISK\" \u003d\u003d \"0\" ]; then"},{"line_number":466,"context_line":"    for IMAGE_TYPE in ${IMAGE_TYPES[@]} ; do"},{"line_number":467,"context_line":"      # We have to do raw last because it is destructive"},{"line_number":468,"context_line":"      if [ \"$IMAGE_TYPE\" \u003d \"raw\" ]; then"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"3aaa91ec_ed433040","line":465,"in_reply_to":"3aaa91ec_4295693e","updated":"2016-06-30 19:45:30.000000000","message":"This is how the other ramdisks avoided the unnecessary image build.  I\u0027m not sure why ironic-agent doesn\u0027t use the same method, but there are some other differences based on IS_RAMDISK that may have been an issue.\n\nLooks like it should be fine to move this into the top-level if though, so I\u0027ll do that.","commit_id":"42bcf1d4df3892d003acc17e33d33e173610b976"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"c42039d2f5b9d301e199a049631e85f467b0eb5e","unresolved":false,"context_lines":[{"line_number":460,"context_line":"unmount_image"},{"line_number":461,"context_line":"cleanup_build_dir"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ no-final-image \u0026\u0026 \"$IS_RAMDISK\" \u003d\u003d \"0\" ]]; then"},{"line_number":464,"context_line":"  has_raw_type\u003d"},{"line_number":465,"context_line":"  for IMAGE_TYPE in ${IMAGE_TYPES[@]} ; do"},{"line_number":466,"context_line":"    # We have to do raw last because it is destructive"}],"source_content_type":"application/x-shellscript","patch_set":4,"id":"bacf61ea_0a21fbfb","line":463,"updated":"2016-07-28 04:08:27.000000000","message":"i do prefer ()\u0027s to make it totally clear what is intended to be negated, but just a nit","commit_id":"8953ab6cdca82a8c0bfd6763f8aece9f99badfe7"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"28053a77e3af47538f09f2adcab66cbddda1ecc3","unresolved":false,"context_lines":[{"line_number":460,"context_line":"unmount_image"},{"line_number":461,"context_line":"cleanup_build_dir"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"if [[ ! $IMAGE_ELEMENT \u003d~ no-final-image \u0026\u0026 \"$IS_RAMDISK\" \u003d\u003d \"0\" ]]; then"},{"line_number":464,"context_line":"  has_raw_type\u003d"},{"line_number":465,"context_line":"  for IMAGE_TYPE in ${IMAGE_TYPES[@]} ; do"},{"line_number":466,"context_line":"    # We have to do raw last because it is destructive"}],"source_content_type":"application/x-shellscript","patch_set":4,"id":"bacf61ea_6127d164","line":463,"in_reply_to":"bacf61ea_0a21fbfb","updated":"2016-07-28 18:08:06.000000000","message":"Good point, I\u0027ll go ahead and make that change.","commit_id":"8953ab6cdca82a8c0bfd6763f8aece9f99badfe7"}]}