Hi Eric, Junio,
thank you very much for your feedback! I honestly apologize I
got so many things wrong.
I'll try to minimize the scope a little bit for the next attempt to
make sure my approach is good first and then expand:
i.e will only teach git branch to delete "-" & give out an
error message in case there is no previous branch
to make sure I got the basics correct.
Elena
On Fri, Mar 18, 2016 at 9:13 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>
> On Fri, Mar 18, 2016 at 12:10 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > On the submitted patch itself, in decreasing order of importance:
> >
> > * The approach you took turns every "-" that appears as a command
> > line argument into "@{-1}", but it does so without even checking
> > where "-" appears on the command line is meant to take a branch
> > name. This closes the door to later add an option that takes "-"
> > as an argument that means something different (e.g. one common
> > use of "-" is to mean "the standard input" when a filename is
> > expected).
> >
> > * There is no explanation and justification in the proposed log
> > message why you took a particular approach. Why is that a good
> > approach? What are the possible downsides? What were the
> > alternatives (if any), and why is the approach chosen is better
> > than them?
> >
> > * We forbid declaration-after-statement in our codebase.
> >
> > * When you do not yet have the "branch I was previously on", I
> > imagine that your implementation would give you this:
> > [...]
> >
> > * You do not need the brace pairs around the body of these new
> > "for" or "if" statements.
>
> Also for consideration: You'd probably also want to add a test or two
> or three to verify that "-" works as intended, and a documentation
> update may be warranted.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html