Code review comment for ~kzapalowicz/snappy-hwe-snaps/+git/udisks2:fix/fail-to-mount-after-unmount

Revision history for this message
Simon Fels (morphis) wrote :

> IMHOP the addition of ciborium to repo's packaging branch was a
> mistake. This probably should have been added to it's own
> upstream branch. We should discuss next week, and if all agree,
> create a card for fixing this.

It's not a mistake as we merged the portions of ciborium we really need on purpose into master. We have modified ciborium already in a way that it doesn't make sense to upstream it also given that there is no upstream anymore. What we have here is a stripped down version of the original ciborium code (mainly just the udisks2 library of it) which has all UI related parts removed and an extra chunk of unit tests added. There is no sense anymore calling this an "upstream" project anymore. We agreed on this a while back when we discussed the addition of ciborium in general.

> As ciborium isn't part of upstream udisks2, it's probably OK to
> live in master, but there are actual changes proposed to udisks2
> code in this merge proposal as well. Was this intentional?

In general master doesn't have to be a packaging-only branch. It is fine to ship code here as well which is specific to the snap and not used elsewhere or coming from an upstream project. As described above portions of ciborium (not ciborium itself) was moved into master as this is really a new, udisks2 snap specific thing, we want to maintain and develop on its own. Because of this it is fine to live in master.

> @Tony, I think you are getting confused by the weird way the
> repo/ciborium seems to be structured.

Go structures its code into packages where each folder becomes its own package. I agree that we maybe should move all Go code into a ciborium subfolder (but lets not call it ciborium as it is not ciborium anymore).

« Back to merge proposal