Overall this looks good. As someone not familiar with the code in question a few comments would have helped. It took me a lot of reads through newFromBazaarRevisions to trace what I think is going on in there. I look forward to a day when we can use namedtuples!
Is there a general frowning on *args? A lot of places needed to be updated to change their calling arguments to be wrapped in a list:
RevisionSet().newFromBazaarRevisions([bzr_revision])
I can't help but wonder if allowing these to come in as
def newFromBazaarRevisions(*revisions):
would end up being a bit cleaner. I've not seen much of that practice so far though.
Overall this looks good. As someone not familiar with the code in question a few comments would have helped. It took me a lot of reads through newFromBazaarRe visions to trace what I think is going on in there. I look forward to a day when we can use namedtuples!
Is there a general frowning on *args? A lot of places needed to be updated to change their calling arguments to be wrapped in a list: ().newFromBazaa rRevisions( [bzr_revision] )
RevisionSet
I can't help but wonder if allowing these to come in as visions( *revisions) :
def newFromBazaarRe
would end up being a bit cleaner. I've not seen much of that practice so far though.