Any comments on the new patch version?
On Fri, Apr 4, 2014 at 10:27 PM, Erik Carstensen <[email protected]>wrote: > Forgot to attach everything in the last mail. > > > On Fri, Apr 4, 2014 at 10:21 PM, Erik Carstensen <[email protected]>wrote: > >> BTW, I noticed that stgit tends to use space around the = in function >> keyword arguments. PEP8 says there should be no space there. Is the >> discrepancy intentional? >> >> On Fri, Apr 4, 2014 at 4:06 AM, Karl Wiberg <[email protected]> wrote: >> >> On Thu, Apr 3, 2014 at 6:54 AM, Erik Carstensen <[email protected]> >>> wrote: >>> >>> > test_expect_success \ >>> > 'No patch' \ >>> > '! stg move --from foo' >>> >>> * It's possible to write tests using far fewer backslashes. See >>> e.g. t/t3300-edit.sh. >>> >> >> Fixed >> >> >>> * Use general_error, command_error, or conflict from test-lib.sh >>> instead of !. >> >> >> Thanks! I also introduced a new function "expect_output". It makes the >> test more readable, and gives you a hint upon failure. >> >> * So moving a patch to the same branch will pop it if necessary, and >>> give it a new autogenerated name? Wouldn't it make more sense to >>> just complain and do nothing? >>> >> >> I was just checking that it doesn't do anything really bad; I thought it >> was easier not to care much about it. But considering your remark on -f >> below, it's easier to forbid it. Done now. >> >> > usage = ['DEST_BRANCH <patch1> [<patch2>] [<patch3>..<patch4>]'] >>> > description = """ >>> > Delete the given patches from the current branch, and add them >>> unapplied to >>> > DEST_BRANCH.""" >>> >> >> Fixed >> >> >>> > def find_unique_name(oldname, blacklist): >>> > # TODO: is this the right place? >>> >>> No, you can delete it because you never call it. But if you did want >>> it (and I actually think this scheme makes sense), then perhaps near >>> make_patch_name in stgit/utils.py. >>> >> >> I removed it. Easier to use the standard scheme. >> >> >>> > if options.src: >>> >>> > iw = None # can't use index/workdir to manipulate another >>> branch >>> > else: >>> > iw = src.repository.default_iw >>> >>> I would have named this src_iw or something, to make it clear which >>> branch it belongs to. >>> >> >> Fixed. Also, changed one use of src_iw; we must use None when pushing in >> order to prevent the conflict from touching the working tree (perhaps this >> is the error in delete as well?) >> >> > > out.info('Created %s on branch %s' % (", ".join(created), >> dest.name)) >> > >> > This line is often going to get very long >> >> Fixed, along with silencing delete (also had to fix a bug there, see >> patch). It now outputs: >> Successfully moved patches: >> master:a -> topic:a-0 >> master:foo -> topic:foo >> >> which brings me to a wild class of ideas on argument syntax: We could >> allow some colon-based notation, yielding a syntax familiar from >> cross-device mv on systems such as Amiga: >> stg move df0:foo df1: >> to move patch foo from branch df0 to branch df1; >> stg move df0:foo df1:bar >> to also rename the patch to bar; >> stg move df0:foo master:bar df1: >> to move two patches from different branches to branch df1 >> >> The latter two cannot be expressed with the current parameter syntax. >> However, the new uses are rather exotic, and the new syntax does not >> provide a natural way to move a remote patch to the current branch without >> naming the current branch. >> >> >>> Also, you don't handle (or test!) the case where -f explicitly >>> specifies the current branch. >> >> >> Fixed now (kind of): I test that giving the current branch is equivalent >> to omitting it, in the test where I check that source and destination >> cannot be the same. >> > >
_______________________________________________ stgit-users mailing list [email protected] https://mail.gna.org/listinfo/stgit-users
