Code review comment for lp:~parthm/bzr/no-chown-if-bzrlog-exists

Revision history for this message
Martin Packman (gz) wrote :

> This certainly looks good to me. Would this approach work on Mac? In the
> python docs[1] the availability of os.open is shown as Unix and Windows. I
> don't know much about Mac so I ask.

The Python docs are a little confusing on this, but by their definition there is no longer such a thing as a mac, anything running OS X just another unix. Upstream Python has pretty much dropped support for everything but posix and windows systems.

> > Raises the question though, is it ever going to be right for a
> > osutils.open_with_ownership function to chown the file if it's *not* just
> > created it? Makes me wonder again whether bundling the chown into the open
> > function makes any sense.

Got my thinking a little backwards here, if chown should only be on create, building that logic into open_with_ownership probably makes sense.

> I agree, I don't see too much use of open_with_ownership in a non-create case.
> I can try to create and test a patch based on the above approach which also
> deals with open_with_ownership. Its used only in one another place IIRC.

Worth pondering how you think the interfaces should work, there are currently three new public functions in osutils, and only as many callsites.

« Back to merge proposal