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

Ankur Sinha (FranciscoD) <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST



--- Comment #10 from Ankur Sinha (FranciscoD) <[email protected]> ---
(In reply to Dominik 'Rathann' Mierzejewski from comment #9)
> Wouldn't it make sense to move
> %{_includedir}/nrnconf.h
> into
> %{_includedir}/%{tarname}
> 
> I don't know where packages that will BuildRequires: neuron-devel expect
> these files to be, so I'll leave this up to you to decide.

I've not seen too many refer to that header so I'll leave it for the time being
and change it later if required.

> 
> Also, I wouldn't define a macro that is longer than what it expands into,
> i.e. I'd just use "nrn" everywhere instead of %{tarname}.

Ah, yes XD

> 
> None of the above are blockers and the package looks good otherwise, so
> approved.

Thanks very much! SCM requested:
https://pagure.io/releng/fedora-scm-requests/issue/9737

Cheers,
Ankur

-- 
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
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]

Reply via email to