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

Change subject: osmo-depcheck: script to verify PKG_CHECK_MODULES
......................................................................


Patch Set 2:

(4 comments)

a few lesser comments for now, need to run now and will continue review later...

https://gerrit.osmocom.org/#/c/10932/2/.gitignore
File .gitignore:

https://gerrit.osmocom.org/#/c/10932/2/.gitignore@8
PS2, Line 8: __pycache__/
technically this would be a separate patch from the rest


https://gerrit.osmocom.org/#/c/10932/2/scripts/osmo-depcheck/buildstack.py
File scripts/osmo-depcheck/buildstack.py:

https://gerrit.osmocom.org/#/c/10932/2/scripts/osmo-depcheck/buildstack.py@2
PS2, Line 2: # (c) 2018 sysmocom - s.f.m.c. GmbH
(You should write "Copyright <year> ...", the (c) is just a gimmick IIUC. I 
typically also append "<[email protected]>")


https://gerrit.osmocom.org/#/c/10932/2/scripts/osmo-depcheck/config.py
File scripts/osmo-depcheck/config.py:

https://gerrit.osmocom.org/#/c/10932/2/scripts/osmo-depcheck/config.py@10
PS2, Line 10: # they are mentioned with PKG_CHECK_MODULES in configure.ac.
indicate whether it's regex or shell glob or ...


https://gerrit.osmocom.org/#/c/10932/2/scripts/osmo-depcheck/config.py@18
PS2, Line 18: repos = {"libosmocore": ["libosmocodec",
would make sense to use tuples instead of lists -- they aren't supposed to be 
mutable, right?

  ["mutable", "list",]
  ["single-item mutable"]

vs.

  ("immutable", "tuple")
  ("single-item tuple",)

general cosmetic: we like to keep lists structured so that changes to them are 
likely to produce single-line patches, i.e. add the final optional comma and 
keep braces separate.

  {
    bla: [
          item,
          item,
         ],
    foo: [
          item,
         ],
   }

(not that important, just saying)



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

Gerrit-Project: osmo-ci
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f495dbe030775f66ac125e60ded95c5d7660b65
Gerrit-Change-Number: 10932
Gerrit-PatchSet: 2
Gerrit-Owner: osmith <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-CC: Neels Hofmeyr <[email protected]>
Gerrit-Comment-Date: Mon, 17 Sep 2018 13:43:06 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Reply via email to