<dustymabe>
jlebon: so other classes don't really override `mutate_image()` as the comment for the function states.. rather they provide a mutate_callback function?
<jlebon>
dustymabe: yeah, exactly
<jlebon>
tbh, I think we should de-OOP this code and make it more functional instead
<jlebon>
it's super hard to follow
<dustymabe>
i was mostly confused because the comment was misleading
<jlebon>
but unless it's done right, we'd end up with an even bigger mess of partially OOP and partially not code
<dustymabe>
i'm trying to find an example where one of the mutate_callback functions uses the image.json data
<dustymabe>
bgilbert: we don't have any set policy that I know of other than i certainly close it if it makes it if the fix makes it to `stable`.
<bgilbert>
ok cool
<jlebon>
dustymabe: i don't disagree, but that first operation will still be costly, and reading image.json on the surface isn't an operation that seems like it should be costly
<jlebon>
in some cases also, you might want it to fail earlier if the ostree content is missing so you don't do unnecessary work
<dustymabe>
jlebon: i'm not sure I understand your argument. It seems as if you think my suggestion will cause unnecessary `import_ostree_commit()` to happen? Do you think that is the case?
<dustymabe>
oh, I see
<jlebon>
dustymabe: i'm saying your suggestion should work, but isn't necessarily objectively better
<dustymabe>
what I am trying to avoid is importing the ostree_commit just for the image.json if you don't need to
<dustymabe>
right now anytime you `mutate_image()` it will get called - whereas it's only really needed for ova.py
<jlebon>
right, i follow. i think you have a point.
<jlebon>
i think in practice though it won't really matter for FCOS and RHCOS pipelines today since all these builds are done in the same job
<jlebon>
but they may not be in the future
bagasse has quit [Ping timeout: 255 seconds]
<jlebon>
(i.e. it'll be a no-op anyway since it's the same workdir where the ostree was composed)
<dustymabe>
I think we are both using the same argument (the no-op) as being a good reason to leave it the way it is AND to change it to the other way
<dustymabe>
in practice the `no-op`/lightweight nature of it makes it so it doesn't really matter when we `import_ostree_commit()`
<dustymabe>
so for that reason i suggest to keep the code cleaner by just putting it alongside the function that needs it (the extract_image_json())
<dustymabe>
either way works
<dustymabe>
let's see what colin thinks
<jlebon>
+1
<dustymabe>
on another note.. i think we are getting closer to adding support for s390x (thanks to some work by lakshmi)
<jlebon>
oh sweet!
ravanelli has joined #fedora-coreos
guesswhat has joined #fedora-coreos
<dustymabe>
saqali: looks like all the release jobs finished
ravanelli has quit [Remote host closed the connection]
ravanelli has joined #fedora-coreos
nemric has joined #fedora-coreos
ravanelli has quit [Remote host closed the connection]
nalind has quit [Quit: bye]
josereyesjdi has joined #fedora-coreos
guesswhat has quit [Quit: Client closed]
mheon has quit [Ping timeout: 260 seconds]
jpn has joined #fedora-coreos
jpn has quit [Ping timeout: 244 seconds]
crobinso has quit [Remote host closed the connection]
josereyesjdi has quit [Ping timeout: 258 seconds]
heldwin has quit [Remote host closed the connection]