[ 
https://issues.apache.org/jira/browse/HBASE-12207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14180617#comment-14180617
 ] 

Dima Spivak commented on HBASE-12207:
-------------------------------------

Some feedback:

- Please use 2 character spaces for indentation (and 4 for line continuations).
- Maybe make the usage message a function (cat with a here document) since then 
you can check for if someone passes too many arguments when parsing command 
line arguments.
- Passing an invalid argument to the script should output the usage message (to 
stderr) and then exit 1.
- Put variable subtitution (e.g. on line 44) in double quotes to protect 
against breaking if spaces are passed.
- On line 81, may want to lowercase everything and check equality instead of 
checking HBASE- or hbase- (in case someone uses Hbase- or some other weird 
combo).
- Consider storing deleted branches in an array instead of concatenating a 
string. This makes it easier to loop through later, as well.
- Put spaces on both sides of pipes throughout to clarify what's going on in 
commands (e.g. line 99).
- Pretty sure you don't need the trailing backslash on line 134 since you're 
inside of quotes, but you may want to just have two echos since in the current 
form, the formatting will be funny (since you have whitespace inside of the 
quotes).
- Try to be consistent and use single brackets for conditionals, if possible 
({{if [ ... ]; then}} versus {{if [[ ... ]]; then}}) for us POSIX nerds. :)
- Put a space before > on line 120.
- Forgive me for being so type A. :-p

It's not strictly related to your commit, by the way, but the ref guide's patch 
section references {{git squash}} as if it's a command that can be run at one 
point. Can you just sort that out (i.e. just say to squash or {{git rebase 
-i}}) since it's relevant to this topic?

> A script to help keep your Git repo fresh
> -----------------------------------------
>
>                 Key: HBASE-12207
>                 URL: https://issues.apache.org/jira/browse/HBASE-12207
>             Project: HBase
>          Issue Type: Improvement
>          Components: documentation, scripts
>            Reporter: Misty Stanley-Jones
>            Assignee: Misty Stanley-Jones
>         Attachments: HBASE-12207-v1.patch, HBASE-12207-v2.patch, 
> HBASE-12207-v3.patch, HBASE-12207-v4.patch, HBASE-12207-v5.patch, 
> HBASE-12207.patch
>
>
> I have a script that does a {code}git pull --rebase{code} on each tracking 
> branch, and then attempts an automatic rebase of each local branch against 
> its tracking branch. It also prompts you to delete local branches for HBASE- 
> JIRAs that have been closed. I think this script may help to enforce good Git 
> practices. It may be a good candidate to be included in dev-support/.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to