tools
tools copied to clipboard
Fix local setup
- Set
$pkginstead of$scriptwith package - Shift arguments
This looks like a reasonable fix to me - is there a reason it hasn't been accepted?
Inadequate justification of the change and explanation of what it does?
Have you tested it? At first glance I'm not sure it does what is expected (but the original looks broken as well).
Sorry Phil, on closer inspection, indeed it does look a bit suspect. On first glance it looked reasonable, but when I look more closely, the original script looks broken in ways I think this patch doesn't address, e.g. these lines:
[ "_$1" = "_-p" -o "_$1" = "_--package" ] && { pkgs="$1"; shift; }
[ "_$1" = "_-b" -o "_$1" = "_--bin" ] && { scripts="$1"; shift; }
e.g. if $1 is -p then pkgs will be set to -p whereas presumably pkgs would be the parameter following the -p or --package argument (and you would expect 2 shifts, not one). It also wasn't completely clear to me what this script is for or how it should be used.
Full disclosure: I was raising a PR myself (#134) and tend to look at open PRs before submitting one, just to see if the repo is actively maintained. I saw this PR from 4 years ago that looked reasonable, so I was curious why nobody had responded (I thought maybe it had accidentally gone unnoticed).
I thought maybe it had accidentally gone unnoticed
There may be an element of that - I can't remember that far back. Unless a patch is obviously correct and useful, or addresses a clearly explained problem, it's easy to put them on one side for later.
As this is a few years back, I don't remember really, but would guess that the script was broken and I fixed its issues and made a low effort PR so maybe the next person would have it working. In the end I've decided against making my own custom RP image and using ansible instead (like on all other systems) and it wasn't important enough to check it again. If there is interest, Ill take a look at it again and improve the documentation of the PR