38 - dest_dict = to_tags.get_tag_dict()
39 - result, conflicts = self._reconcile_tags(source_dict, dest_dict,
40 - overwrite)
41 - if result != dest_dict:
42 - to_tags._set_tag_dict(result)
43 + master = to_tags.branch.get_master_branch()
44 + try:
45 + if master is not None:
46 + master.lock_write()
47 + conflicts = self._merge_to(to_tags, source_dict, overwrite)
48 + if master is not None:
49 + conflicts += self._merge_to(master.tags, source_dict,
50 + overwrite)
51 + finally:
52 + if master is not None:
53 + master.unlock()
Isn't this better written as:
master = to_tags.branch.get_master_branch()
if master is not None:
master.lock_write()
try:
conflicts = ...
finally:
master.unlock()
Note that the way you wrote it, if we fail to take the write lock in the first place (no such permission), we end up still calling unlock.
I think you wrote it this way because you check the conflicts anyway, but the code looks clearer to me if you just add an 'else: conflicts = ...' to the above statement.
I suppose either way it is a bit clumsy, since you want the merge to fail immediately if you can't propagate the tags to the master branch before you try to merge to the local one...
38 - dest_dict = to_tags. get_tag_ dict() _tags(source_ dict, dest_dict, _set_tag_ dict(result) branch. get_master_ branch( ) to(to_tags, source_dict, overwrite) to(master. tags, source_dict,
39 - result, conflicts = self._reconcile
40 - overwrite)
41 - if result != dest_dict:
42 - to_tags.
43 + master = to_tags.
44 + try:
45 + if master is not None:
46 + master.lock_write()
47 + conflicts = self._merge_
48 + if master is not None:
49 + conflicts += self._merge_
50 + overwrite)
51 + finally:
52 + if master is not None:
53 + master.unlock()
Isn't this better written as:
master = to_tags. branch. get_master_ branch( ) lock_write( ) unlock( )
if master is not None:
master.
try:
conflicts = ...
finally:
master.
Note that the way you wrote it, if we fail to take the write lock in the first place (no such permission), we end up still calling unlock.
I think you wrote it this way because you check the conflicts anyway, but the code looks clearer to me if you just add an 'else: conflicts = ...' to the above statement.
I suppose either way it is a bit clumsy, since you want the merge to fail immediately if you can't propagate the tags to the master branch before you try to merge to the local one...
I think the logic is otherwise ok.