Change in osmo-dev[master]: replace src/* git scripts with a single src/gits

historical

Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11560 )

Change subject: replace src/* git scripts with a single src/gits
......................................................................

Patch Set 4:

(13 comments)

patch set coming up...

https://gerrit.osmocom.org/#/c/11560/4/src/gits
File src/gits:

https://gerrit.osmocom.org/#/c/11560/4/src/gits@2
PS4, Line 2: argparse
> argparse is not used. […]
the reason why argparse will not work so well is passing arguments to git. consider:

  gits clean -dxf

then 'gits' will argparse the -dxf arguments, but they were intended to be passed on to each 'cd subdir/; git clean -dxf' cmdline.

good catch about -h

https://gerrit.osmocom.org/#/c/11560/4/src/gits@11
PS4, Line 11: re_status_mods = re.compile('^\t(modified|deleted):.*')
> This is only used once in the code. […]
I think I'd rather precompile all others here as well

https://gerrit.osmocom.org/#/c/11560/4/src/gits@29
PS4, Line 29:     print(' '.join(cmd))
> How about differentiating the command from the output it produces? For example with a leading '>' ch […]
can do. I think I'd use the '+ ' ala 'set -x'

https://gerrit.osmocom.org/#/c/11560/4/src/gits@37
PS4, Line 37:     sys.stderr.flush()
> why flush the output only when the section_marker is needed?
not sure. probably came from previous code... should probably flush above instead

https://gerrit.osmocom.org/#/c/11560/4/src/gits@45
PS4, Line 45: m
> how about "branch" as variable name?
it's a re "match object"

https://gerrit.osmocom.org/#/c/11560/4/src/gits@48
PS4, Line 48:   return m.group(1)
this one is the branch

https://gerrit.osmocom.org/#/c/11560/4/src/gits@68
PS4, Line 68: up to date
> With git 2.11.0 I get: […]
yeah, that's the main ultra drawback of this script so far. I really really shouldn't use porcelain. I'm not sure how to use plumbing though, so far it's "works-for-me" -- do you know how to fix that?

https://gerrit.osmocom.org/#/c/11560/4/src/gits@122
PS4, Line 122:     strs.append(''.join(branch_info))
> why make a list first, and then join it?
that's my common string composition pattern. If I do "str = str + more", then I create an ever growing string in a loop. If I append to a list and later join, I only create one large string in the end. Well, it's premature optimization, but I prefer that style.

In this instance it was about conditionally adding strings. a bit overboard, yes

https://gerrit.osmocom.org/#/c/11560/4/src/gits@194
PS4, Line 194:     print('\n===== %s =====' % git_dir)
> If we print out something like "> cd %s && %s" before the output of the command, it would be consist […]
I like more visible markers in this case

https://gerrit.osmocom.org/#/c/11560/4/src/gits@200
PS4, Line 200: SkipThisRepos
> How about "SkipThisRepo"? repo is commonly used to abbreviate repository, so when reading "repos" I' […]
I've always thought of "repos", not "repo", had the same "conflict" with stsp many years ago... :)

https://gerrit.osmocom.org/#/c/11560/4/src/gits@223
PS4, Line 223:       if v == answer:
             :         return answer
             :       if v == '*':
             :         return answer
> how about: […]
I see, but I prefer listing each valid_answers case in its separate 'if'

https://gerrit.osmocom.org/#/c/11560/4/src/gits@359
PS4, Line 359:     cmd_do(sys.argv[1:])
> I'm not sure if it's a good idea to perform "do" by default - whenever there's a typo in any of the  […]
but I want to write 'gits fetch' :)
and don't want to write a shim for each and every one of those...

you're right of course, maybe a shim then.

https://gerrit.osmocom.org/#/c/11560/4/src/gits@362
PS4, Line 362: # vim: shiftwidth=2 expandtab tabstop=2
> Since there are not Python coding guidelines in the wiki yet: […]
wow, you're just as obsessed with details like I am!

-- 
To view, visit https://gerrit.osmocom.org/11560
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-dev
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I579e7af26d76d5c5d83b2349695456bc7b54f5a2
Gerrit-Change-Number: 11560
Gerrit-PatchSet: 4
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Comment-Date: Tue, 06 Nov 2018 15:31:09 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181106/c16f2728/attachment.htm>