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



--- Comment #22 from Tadej Janež <[email protected]> ---
(In reply to Ryan from comment #20)
> Hi, thanks for the review!
> 
> > > BuildRequires:  automake
> > 
> > I don't think it is a good practice to recreate the project's scripts by 
> > running ./automake.sh below?
> > I would rather just BuildRequire make.
> > 
> > > %autosetup -p1
> > 
> > Since there are no patches, it should be enough to just call "%autosetup".
> > 
> > > %build
> > > ./automake.sh
> > 
> > As said above, I would remove this line and just keep the "%configure" and 
> > "%make_build".
> 
> There isn't a configure script in the tarballs for par2cmdline-turbo, so I
> put that in there to generate it.
> Is there a better way?

Oh, I see now. I didn't realize that par2cmdline-turbo's upstream diverged here
and doesn't provide release tarballs that would pre-generate the configure
script and include it in the tarball.

Then let's leave it as is and please file an issue to upstream and ask them to
consider creating release tarballs which include the configure script to ease
the installation experience and improve consistency across various systems.

> 
> > Can you add a comment line here above to explain why this is being done?
> > E.g.: # Renaming binaries and man page to avoid conflicts with the 
> > par2cmdline package.
> 
> > Removing %{buildroot}%{_bindir}/par2 can be skipped since it has been 
> > renamed to par2turbo in the mv command above.
> 
> Both done :)
> 
> > In the next step, it would make sense to file an issue with the upstream 
> > and ask them to rename things there.
> 
> Here: https://github.com/animetosho/par2cmdline-turbo/issues/49

Thanks!
I've followed up and commented there myself.

Could you update the package to version 1.4.0 that has been released in the
mean time?
I think we should be in good position to add it to Fedora then.


-- 
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=2231216

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

-- 
_______________________________________________
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