https://bugzilla.redhat.com/show_bug.cgi?id=2218306 Ben Beasley <code@xxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(code@musicinmybra | |in.net) | --- Comment #4 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- (In reply to Roman Inflianskas from comment #3) > @code@xxxxxxxxxxxxxxxxxx > > I checked the spec file. It looks good. However: > 1. I couldn't check it with `fedora-review` (see: > https://bugzilla.redhat.com/show_bug.cgi?id=2217496). Review template link > in the automatic message above is broken (probably, because of the same > reason). The workaround mentioned in that bug of doing the review in a Fedora 38 chroot, “fedora-review -b 2218306 -m fedora-38-x86_64”, should be adequate. > 2. I believe I miss some knowledge (and I couldn't acquire it by searching > on web). Could you please explain, why did you use variable `${ignore-} > instead of defining a macro? Also, AFAIK, some shells do not allow `-` in > variable names; how does this work and why does this mean? This is a pattern/habit I’ve picked up for ignoring files in pytest. I build up a shell variable called “ignore,” each time without caring whether the shell variable is set yet or not, and then expand it (unquoted) in the %pytest invocation. That way, I can add additional lines of the same form, reorder them, or comment out any or all of them freely. See https://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_02 for documentation on "${foo-}"; it expands to the empty string if foo is unset, but is more explicit than "${foo}" when foo might be unset, and works even if the “nounset” shell option is set (https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#set). All of this is POSIX sh functionality, so it will work in any shell that attempts to be POSIX-compliant (bash, dash, ash, ksh, etc.). > ignore="${ignore-} --ignore=tests/test_docs.py" > TZ=utc %pytest -v ${ignore-} For a simple case like this, the above is exactly equivalent to > TZ=utc %pytest -v --ignore=tests/test_docs.py which would also be fine, and which is probably what I would have written if I hadn’t established such a strong habit from messier packages. I’m happy to change it for simplicity, although I think it doesn’t really matter much either way. I use a similar pattern to build up the argument for the -k option when I need it: > k="${k-}${k+ and }not test_foo" > k="${k-}${k+ and }not test_bar" > k="${k-}${k+ and }not test_bat" > %pytest -k "${k-}" This, too, is just POSIX sh, although we can safely assume that the shell in spec files for Fedora is Bash. I do still personally prefer to avoid bashisms in spec files unless they really do make things a lot simpler. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2218306 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202218306%23c4 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
