[Bug 1264546] Review Request: soletta - A framework for making IoT devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #5 from Gustavo Lima Chaves <gustavo.lima.chaves@xxxxxxxxx> ---
> I can't sponsor you, but I can give some suggestions.

Thanks a lot for the suggestions, Christopher! I guess Fidencio and I
will continue fine with this (thank you too, Panda!).

>
> 1. Please include your email in changelog.

Done. I really wish I could have a single, initial changelog for an
initial packaging attempt. Is that possible?

>
> You can try rpmdev-bumpspec to see what exactly is changelog, obviously the
> current one is poor.
>
> 2. "%define soletta_major 0
> %define soletta_minor 0
> %define soletta_build 1
> %define soletta_release beta5"
>
> https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

Thanks a lot, I won't forget.

>
> 3. Drop Group tags in all packages.

Okay, better for me.

>
> 4. It's better to put %post(un/trans) after %install/%check but before %files.

Done.

>
> 5. "This package contains the sysctl linux-micro module for %{name}. The
> module sets kernel parameters from sysctl.conf files. This service
> will mimic systemd-sysctl.service and read the settings from
> '/etc/sysctl.conf' or '/run/sysctl.d', '/etc/sysctl.d',
> '/usr/local/lib/sysctl.d', '/usr/lib/sysctl.d', '/lib/sysctl.d'. Files
> are processed in alphabetical order. See
> http://www.freedesktop.org/software/systemd/man/systemd-sysctl.service.html.
> "
>
> Sorry, we don't use some of these paths. Please remove them.

All linux-micro modules were moved to the base lib package, so these
texts are no more. I did that because linux-micro would eventually
vanish in a f23 version of the spec.

>
> 6. The build log is silent, I could only see
>
> GEN *
> CC *
> LD *
> BIN *

Well, by default our build log IS indeed silent. You can see all
that's happening with V=1, if you wish.

>
> This makes it impossible to detect if it's been built correctly:
>
> https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
>
> You must compile packages with %optflags and %__global_ldflags for linking,
> note these need to be inserted, but not substituted of all flags since you
> might use some custom flags as well.

Nevertheless, I added those, thanks. The only "strange" output you'll
see there are cpio errors when trying to generate debug symbols for
the duktape (JavaScript) sub-module. There's an open ticket for the lib
authors to remove #line directives from their released code, that should
vanish soon (next release, maybe).

>
> 7. Drop %defattr(-, root, root, -)

Done.

>
> 8. %{_includedir}/soletta/*
>
> No, you forgot %{_includedir}/soletta/ itself, this will only include files
> underneath the dir without dir itself.

Ok, various %dir added.

>
> 9. Same as above, %{_datadir}/soletta and %{_libdir}/soletta weren't included,
> but since different subpackages put files inside, you need to decide on your
> own.

Ditto.

>
> 10. You use %license, but put it in lib%{name}-pin-mux-module-Edison only,
> that's wrong, you should put it into lib%{name}. (because every subpkg depends
> on it based on the spec from my view)

Now it happens just after the main lib's files.

>
> 11. I don't see any packages with name %{name}, so isn't it better to rename
> this spec to libsoletta?

I intend to package other things on the soletta project umbrella here.
Namely https://github.com/solettaproject/soletta-dev-app is the next
candidate, so...

>
> 12 Last question, do we really need such many subpackages?

11 of them were merged with the main lib now. The others I'd like to
keep -- they are optional for most use cases and may require different
deps.

Again, thanks a lot for the review. I'll have this new spec uploaded
soon for you to see.

-- 
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
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]