These are unofficial review comments.

You referred to this link :

http://devel.ringlet.net/editors/hexer/CHANGES

explaining the reason of your patch, but the file in this link is not updated, and doesn't have any changes mentioned for version 0.1.8

Moreover, explaining the reason for your patch near Patch0 field would be better, in my humble opinion.

There is a superfluous space before the start of the changelog entry.

* Fri Apr 17 2015 Alex Kashchenko <mail> 0.1.8-1
 - initial package, in sigaction struct sa_mask field is zeroed using
   sigemptyset function instead of assigning zero (prohibited in gcc5),
   upstream changelog link: http://devel.ringlet.net/editors/hexer/CHANGES

Space before '-'

rm -rf %{buildroot} at the beginning of the install is not required. You can remove it.

Addressed comments 1, 2 and 3 (please remove "_v1" postfix after download):

Spec URL: https://bitbucket.org/akasko/share/downloads/hexer.spec_v1
SRPM URL: https://bitbucket.org/akasko/share/downloads/hexer-0.1.8-1.fc23.src.rpm_v1

Diff between v0 and v1 specs: https://bitbucket.org/akasko/share/downloads/v1_spec.diff


Comment 1:

 - removed upstream CHANGES link because this link (http://devel.ringlet.net/editors/hexer/CHANGES) is not up to date, and while up to date link exists in VSC (https://www.gitorious.org/hexer/hexer/source/ed78162928ba92dc455e5bf1a3d05914bd993bae:CHANGES), it is going to become unavailable soon after gitorius.org shutdown
 - moved patch description next to Patch0 entry as a comment
 - added the comment about patch upstreaming

Comment 2:

 - removed superfluous space in changelog

Comment 3:

 - removed rm -rf %{buildroot} entry


RPMLint output:

    rpmlint hexer-0.1.8-1.fc23.src.rpm 
    1 packages and 0 specfiles checked; 0 errors, 0 warnings.
    rpmlint hexer-0.1.8-1.fc23.x86_64.rpm
    1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Try pointing the fedora-review tool at this ticket: fedora-review -b 1213065
It evaluates the "Spec URL:" and "SRPM URL:" lines, downloads the latest packages, performs local test-builds and many checks related to the packaging guidelines.


> please remove "_v1" postfix after download

The better choice would have been to practice increasing "Release" for each package update:

  https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes

 | Increase the "Release" tag every time you upload a new package to avoid
 | confusion. The reviewer and other interested parties probably still have 
 | older versions of your SRPM lying around to check what has changed between
 | the old and new packages; those get confused when the revision didn't 
 | change. 


> License: BSD

A modified 3-clause BSD that adds a specific requirement for documenting changes in the README and the source files.

Not really a hurdle for the current patch but can be easy to miss.


> cc -O -DHEXER_VERSION=\"0.1.8\"  -c -o buffer.o buffer.c

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

rpmlint:
hexer-debuginfo.x86_64: E: debuginfo-without-sources

akasko's scratch build of hexer-0.1.8-1.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12184492

Updated package to upstream version 0.2.3:

Spec URL: https://bitbucket.org/akasko/share/downloads/hexer.spec
SRPM URL: https://bitbucket.org/akasko/share/downloads/hexer-0.2.3-1.fc24.src.rpm

Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=13378191

RPMLint output:

rpmlint SRPMS/hexer-0.2.3-1.fc24.src.rpm 
> 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
rpmlint RPMS/x86_64/hexer-0.2.3-1.fc24.x86_64.rpm 
> 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
rpmlint RPMS/x86_64/hexer-debuginfo-0.2.3-1.fc24.x86_64.rpm 
> 1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comments addressed:

1) Release tag: In this update changed the upstream version, will increase Release for the subsequent changes

2) License BSD: license name was taken from this table - https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses , please let me know if any changes are required here

3) compiler flags: added export CFLAGS="%{optflags}" before calling make

4) debuginfo: in 0.2.3-1 debuginfo RPM passes rpmlint

Hello!

I'm runnign the review now. The apckage is looking more then good.

The most painful issue are remaining the license and code.

Indeed the license in files and in copyright looks like BSD. And they probably are. Also the tarball provided as source tarball looks like open sources :)
I' would like to see the license more clearly, but from my side, I'm happy with current state of things. Also I would like to see some publicly accessible repository... But the open release tarball looks good enough for me.

Have I overlooked something?

I will publish final review soon.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "BSD (3 clause)", "Unknown or generated". 7 files have unknown
     license. Detailed output of licensecheck in
     /home/jvanek/1213065-hexer/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in hexer-
     debuginfo
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: hexer-0.2.3-1.fc23.x86_64.rpm
          hexer-debuginfo-0.2.3-1.fc23.x86_64.rpm
          hexer-0.2.3-1.fc23.src.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (debuginfo)
-------------------
Checking: hexer-debuginfo-0.2.3-1.fc23.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
2 packages and 0 specfiles checked; 0 errors, 0 warnings.



Requires
--------
hexer-debuginfo (rpmlib, GLIBC filtered):

hexer (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libm.so.6()(64bit)
    libncurses.so.5()(64bit)
    libtinfo.so.5()(64bit)
    rtld(GNU_HASH)



Provides
--------
hexer-debuginfo:
    hexer-debuginfo
    hexer-debuginfo(x86-64)

hexer:
    hexer
    hexer(x86-64)



Source checksums
----------------
http://devel.ringlet.net/editors/hexer/hexer-0.2.3.tar.xz :
  CHECKSUM(SHA256) this package     : 3bd78643f307f5c7a9a814041c7f02fd88c2a1b81b22bd63b0c61bccad98ae1e
  CHECKSUM(SHA256) upstream package : 3bd78643f307f5c7a9a814041c7f02fd88c2a1b81b22bd63b0c61bccad98ae1e


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -b 1213065 -m fedora-23-x86_64
Buildroot used: fedora-23-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

There are sources of some "calc" even with manpage. Is it intentionally not-packed?
It seems that makefile is supporting setupable bindir and mandir. Maybe it is better to set them in make and make install rather then cp?
Have you tried arm32 build?

One note, where I don't know how the world is standing - Have you considered moving .h files to devel subpackage? I guess there is no need to do it, unles syou really can use it as library.

Anyway the package is ok from me.

This package is APPROVED.

Thanks for the review, on the comments:

> There are sources of some "calc" even with manpage. Is it intentionally not-packed?

Yes, it is a separate utility myc that is not built by default. It's code is also built into the main hexer binary.

> It seems that makefile is supporting setupable bindir and mandir. Maybe it is better to set them in make and make install rather then cp?

"install" target depends on "all" target in that Makefile - so it is not convenient to use it inside the "%install". As only two files are required to be copied, current way looks more straightforward.

> Have you tried arm32 build?

Yes, armv7hl build works.

> One note, where I don't know how the world is standing - Have you considered moving .h files to devel subpackage? I guess there is no need to do it, unles syou really can use it as library.

It looks like it is not intended to be used as a library.

(In reply to Alex Kashchenko from comment #12)
> Thanks for the review, on the comments:
> 
> > There are sources of some "calc" even with manpage. Is it intentionally not-packed?
> 
> Yes, it is a separate utility myc that is not built by default. It's code is
> also built into the main hexer binary.

I see. If you will ever pack it, I would recommend to prefix it to hexer-calc (And same wit manpage)

But its completely up to you.

if you don't mind, I will comaintaint this package with you and observe the changes you are doing for some time.
> 
> > It seems that makefile is supporting setupable bindir and mandir. Maybe it is better to set them in make and make install rather then cp?
> 
> "install" target depends on "all" target in that Makefile - so it is not
> convenient to use it inside the "%install". As only two files are required
> to be copied, current way looks more straightforward.

np.

> 
> > Have you tried arm32 build?
> 
> Yes, armv7hl build works.

thanx:)
> 
> > One note, where I don't know how the world is standing - Have you considered moving .h files to devel subpackage? I guess there is no need to do it, unles syou really can use it as library.
> 
> It looks like it is not intended to be used as a library.

Sure.

hexer-0.2.3-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-7d69fa003b

hexer-0.2.3-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-7d69fa003b

hexer-0.2.3-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.