https://bugzilla.redhat.com/show_bug.cgi?id=2385991



--- Comment #19 from Maxwell G <[email protected]> ---
Thanks for working on this and responding to the feedback. Let me know when
you're ready for another review :).

> Regarding the RPM macros and my choice of MIT, I actually went with Fedora's 
> licensing for spec files and packaging-related deliverables.

Yeah, that makes sense to me, you just have to include the MIT license text
with %license in the actual package to stay in line with the Licensing
Guidelines.

> This patch comes from the master branch, so I didn't feel the need for a 
> comment.

It's still good to have *something*, even a one sentence "This patch was
backported from upstream." comment would suffice.

> This looks like a side effect of patching fedora-review to feed it the SRPM's 
> spec instead of a separate one. I'm not sure my own patch had this problem, I 
> don't remember.

I assumed it was because I used the -r/--rpm-spec and/or -p/--prebuilt
fedora-review flags. I don't think it had to do with your patch.

>> - I see there's a `# XXX: HARETEST_INCLUDE=slow` comment. What's the story 
>> there?
>
> I didn't want to waste time waiting for tests feeding GBs of zeros to hash 
> functions while I was iterating, so I kept a reminder.

I would suggest a bcond here. At the top of the specfile, you could include

```
# Enable this to run the slow tests
%bcond slow_tests 0
```

and then use

```
%make_build check %{?with_slow_tests:HARETEST_INCLUDE=slow}
```

to only run the slow tests when the slow_tests bcond is enabled.

> Thanks for the tip. I literally have no preference, this is my very first lua 
> scripting in a spec file. I'm a total noob in this area.

You're also welcome to use normal shell scripting here if you prefer that
approach.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2385991

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202385991%23c19

-- 
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam, report it: 
https://forge.fedoraproject.org/infra/tickets/issues/new

Reply via email to