Please use plain links to the files them selves, such as:

Spec URL: https://gist.githubusercontent.com/mkulik-rh/0899b8f07bb6fabeee0c7a94e7b8d340/raw/d448ac69bfac6448fc3462e3f5605ba9b04d87f1/pg_auto_failover.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/mkulik/pg_auto_failover/fedora-rawhide-x86_64/02562629-pg_auto_failover/pg_auto_failover-1.6.1-1.fc36.src.rpm

Otherwise various tools don't work, as they cannot download the files, and get some html file rather then the spec file.

Thanks, already fixed. I will remember for the future.

I will be the Reviewer of this package.

I've ran the 'fedora-review' tool on this package.

Let's begin with the big findings first:
------------------------------------------

'fedora-review' reports:
  Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files.
  Note: Documentation size is 3205120 bytes in 169 files.
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Reviewer opinion:
  The package manual page coverage is rather extensive for a single tool.
  Even if the HTML Docs won't be present, I'd be quite confident to say the package is still well documented when installed.
  
  The additional 3MB in size HTML Docs should be moved to a standalone sub-package.
    If the Docs is architecture-independent, the sub-package should be 'noarch'.

  Q: Does the HTML docs provide more / different information or it is just a duplicate of the manpages in a different format ?
     If the Docs would be just a duplicate, I'd consider not packing it.
  Also, please note the Docs contains code under additional licenses, e.g. JQuery code under MIT license.
  Since the Docs utilise CSS, JS, JQuery, ... it requires an application capable interpreting those, to be used at all.
    Might be a good idea to take a look at other packages to learn how they deal with Docs in such format.

------------------------------------------

'fedora-review' reports:
  Rpmlint
  -------
  W: manpage-not-compressed gz /usr/share/man/man1/pg_autoctl config check.1

Reviewer opinion:
  The manpages should be compressed.
  Also, we have automation for that. Take a look at the second paragraph describing usage of the RPM "%doc" directive and "%_pkgdocdir" macro:
    https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

------------------------------------------

'fedora-review' reports:
  Rpmlint
  -------
  W: invalid-license Apache

Reviewer opinion:
  Here is a list of approved licenses for Fedora:
    https://fedoraproject.org/wiki/Licensing:Main
  The error might be caused by using incorrect name or abbreviation for the same license.

------------------------------------------

Reviewer finding:
  The SPECfile:
  |  # openssl is required for ssl-self-signed option in pg_auto_failover
  |  Requires:       postgresql postgresql-contrib openssl glibc

Reviewer opinion:
  Please take a look at:
    https://docs.fedoraproject.org/en-US/packaging-guidelines/#_dependency_types
  and consider making some of the "Requires:" to be "Recommends:" or just "Suggests:" only.
  The comment suggest that the "openssl" is a good candidate for such change.

  Also, are you sure the 'glibc' dependency is necessary? That would be most unusual.
  If required by the binaries, it should be required automatically. In such case you even MUST NOT duplicate such automatic requirement.

------------------------------------------

Reviewer finding:
  The SPECfile:
  |  %files
  |  %{_bindir}/*
  |  %{_libdir}/*

Reviewer opinion:
  Please do not use Globs for a critical parts of the package.
  While it is not forbidden for most parts of the package, it is a best practice to not conceal the important files that are actually packed.
  Explicit listing also serves as a good protection mechanisms for the maintainer during package rebases - to catch unexpected files additions or removals.

Update:

New SPEC file: https://gist.githubusercontent.com/mkulik-rh/0899b8f07bb6fabeee0c7a94e7b8d340/raw/85408546dcf5364dfa2c2ee9790dac6095e6972a/pg_auto_failover.spec

New RPM builds: https://download.copr.fedorainfracloud.org/results/mkulik/pg_auto_failover/fedora-35-x86_64/02676689-pg_auto_failover/pg_auto_failover-1.6.1-1.fc35.src.rpm

Changes:

- Moved HTML docs to separate package (docs)
- Fix license for main and sub-packages
- Fix manual(s) auto gz compression
- Change Globs in %files macro to static ones
- Move openssl package to Recommends, remove glibc


Docs files contain the same information as man files but they also provide some additional information:
- architecture-multi-standby.html
- failover-state-machine.html
- security.html

I'd still make the %files section more clear to catch possible unexpected changes in the future.

Instead of:
---
%files
%{_bindir}/pg_autoctl
%{_libdir}/pgsql
%{_datadir}/pgsql
%{_mandir}/man1/*
%{_mandir}/man5/*
%license LICENSE
---

I suggest:
---
%files
%{_bindir}/pg_autoctl

%dir %{_libdir}/pgsql
%{_libdir}/pgsql/pgautofailover.so
%{_libdir}/pgsql/bitcode

%dir %{_datadir}/pgsql
%dir %{_datadir}/pgsql/extension
%{_datadir}/pgsql/extension/pgautofailover--*.sql
%{_datadir}/pgsql/extension/pgautofailover.control

%{_mandir}/man{1,5}/*.gz

%license LICENSE
---

======================

I googled whether this package has already been packed by anyone to seek inspiration and avoid re-inventing the wheel.
I found that Devrim Gündüz <devrim> is a maintainer of this package in CentOS 7 and he is keeping it up-to-date.
  https://centos.pkgs.org/7/postgresql-13-x86_64/pg_auto_failover_13-1.6.1-2.rhel7.x86_64.rpm.html

He moved the bytecode to a separate package, with a single purpose of JIT.
That sounds like a good idea to me.
Though I would like first to be sure it is possible to use the JIT files in Fedora in the first place.
I don't see a point in packing files without purpose.
Please:
1/ examine if and how can be those JIT files used in Fedora
2/ move them to a separate sub-package, remove them entirely, or make a different decision;  leave comment in the SPECfile in any case.

You may even try to reach Devrim and ask him, so the package maintenance efforts can be coordinated.

Please, also take a look at the:
  "PG_CONFIG=%{pginstdir}/bin/pg_config"
variable definition before "make" and "make install" commands and find out whether they are useful in our case.

Updated SPEC: https://gist.githubusercontent.com/mkulik-rh/0899b8f07bb6fabeee0c7a94e7b8d340/raw/bca577e92d84706ecc5176fe299dcfe9a1623c94/pg_auto_failover.spec
Updated SRPM: https://download.copr.fedorainfracloud.org/results/mkulik/pg_auto_failover/fedora-rawhide-x86_64/02679427-pg_auto_failover/pg_auto_failover-1.6.1-1.fc36.src.rpm


We don't need to specify PG_CONFIG variable for a build process in our case, path to pg_config is detected automatically.
This variable might be used when multiple version of postgresql are installed, to select one.

- I cleanup BuildRequires and %files.
- I added global macros to control subpackages builds.
-I separated JIT extension to subpackage. I don't see scenario when JIT code might be preferred 
 for this package by postgresql but ofc It can be forced to use. Regardless.. we should provide that option since we have postgresql-llvmjit

Linter RC file:

Updated SPEC: https://gist.githubusercontent.com/mkulik-rh/0899b8f07bb6fabeee0c7a94e7b8d340/raw/2a4bb6d96f11265fcfb622869b128594f374142c/pg_auto_failover.spec
rpmlintrc: https://gist.githubusercontent.com/mkulik-rh/ff1be946ae9060b440be1c99cf9b84a4/raw/30819eaf183cd7b8d137115f5d77ee7f6ab623d0/pg_auto_failover.rpmlintrc

- Add linter exceptions for docs and llvmjit subrpms.
- Cleanup spec file

All packages seems to work correctly. I wrote short HOWTO to setup and test basic functionality of pg_auto_failover and JIT extension for it:

######################
#  pg_auto_failover  #
# Setup & test HOWTO #
######################

# First we need to install pg_auto_failover
# postgres will be installed automatically as dependency
# RPMs links can be found above
> dnf install pg_auto_failover-1.6.1-1.fc34.x86_64.rpm
> dnf install pg_auto_failover-llvmjit-1.6.1-1.fc34.x86_64.rpm

# Let's create default database (we are not gonna use it)
> postgresql-setup --initdb

# Let's try to start postgres server
> systemctl start postgresql

# Let's install screen, we will use it later
> dnf install -y screen

# JIT should be enabled by default after -llvmjit installation (Default ON since postgresql version 12)
> su postgres
> psql
> SHOW jit;



# When we su to another account (postgres) from root we need to adjust this variables
> export XDG_RUNTIME_DIR="/run/user/26" # <-- make sure ID is correct (postgres user)
# make sure that root directories in PATH after su are gone
> export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin

# Let's setup DIRs for monitor node
> export PGDATA=/tmp/monitor
> export PGPORT=5000

##################################
# pg_auto_failover setup monitor
##################################

# let's open screen session (as postgres user)
> screen
# adjust variables
> export PGDATA=/tmp/monitor
> export PGPORT=5000
> export XDG_RUNTIME_DIR="/run/user/26"
> export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin

# create monitor instance
> pg_autoctl create monitor --ssl-self-signed --hostname localhost --auth trust --run

# detach screen CTRL +a +d

##################################
# pg_auto_failover setup node 1
##################################

# let's open another screen session for node 1 ( postgres user )
> screen
# adjust variables
> export XDG_RUNTIME_DIR="/run/user/26"
> export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
> export PGDATA=/tmp/node_1
> export PGPORT=5001

# create node_1 instance
> pg_autoctl create postgres \
>    --hostname localhost \
>     --auth trust \
>    --ssl-self-signed \
>    --monitor 'postgres://autoctl_node@localhost:5000/pg_auto_failover?sslmode=require' \
>    --run

# detach screen CTRL +a +d

##################################
# pg_auto_failover setup node 2
##################################

# let's open another screen session for node 2 (postgres user)
> screen
# adjust variables
> export XDG_RUNTIME_DIR="/run/user/26"
> export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
> export PGDATA=/tmp/node_2
> export PGPORT=5002

# create node_2 instance
> pg_autoctl create postgres \
>     --hostname localhost \
>    --auth trust \
>     --ssl-self-signed \
>    --monitor 'postgres://autoctl_node@localhost:5000/pg_auto_failover?sslmode=require' \
>    --run

# detach screen CTRL +a +d

# Now all is setup and we can test pg_auto_failover from postgres user
# We need to remember to adjust all variables when we use su (this can be automated in init scripts)
# Example commands to test:
> pg_autoctl show state
> pg_autoctl perform switchover

Result:
> $ pg_autoctl show state
>  Name |  Node |      Host:Port |       LSN | Reachable |       Current State |      Assigned State
> -------+-------+----------------+-----------+-----------+---------------------+--------------------
> node_1 |     1 | localhost:5001 | 0/30000D8 |       yes |             primary |             primary
> node_2 |     2 | localhost:5002 | 0/30000D8 |       yes |           secondary |           secondary

# Testing JIT extension require some changes.
# We need to fake one thing because running JIT is not really fast/optimal in most scenarios:
# we need to change config settings in postgresql monitor node ( for example )
# to force postgres to use JIT when looking at cost of a Query.
# We have 2 options to do that.

##########
# OPTION 1
##########
# Edit postgres configuration file in /tmp/monitor

# Change this options: 
# jit_above_cost = 1
# jit_inline_above_cost = 1
# jit_optimize_above_cost = 1

# After setting this up we need to resume screen with monitor
# and rerun it.
> screen -r <pid monitor>

# To kill current command:
> CTRL+C

# rerun the same command
# Proper command here would be probably: pg_autoctl start monitor
# but this is fine to ( it will skip creating)
> pg_autoctl create monitor --ssl-self-signed --hostname localhost --auth trust --run


##########
# OPTION 2
##########

# Execute as postgres user
> psql -h localhost -p 5000
> set jit = on;
> set jit_above_cost = 1;
> set jit_inline_above_cost = 1;
> set jit_optimize_above_cost = 1;

# Do not close connection, otherwise this settings will be lost


# Here we should be still connected to monitor postgres node after option 2 ( via psql )
# if we choose option 1 we need to connect to it:
> psql -h localhost -p 5000


# Lets create pg_auto_failover extension
> CREATE EXTENSION pgautofailover CASCADE;

# Let's try to execute any command with pg_auto_failover function with analyze:
> explain analyze select * from pgautofailover.get_nodes();

# Expected result:
                                                   QUERY PLAN                                                    
> -----------------------------------------------------------------------------------------------------------------
>  Function Scan on get_nodes  (cost=0.00..10.00 rows=1000 width=85) (actual time=850.328..850.328 rows=0 loops=1)
>  Planning Time: 0.033 ms
>  JIT:
>   Functions: 2
>   Options: Inlining true, Optimization true, Expressions true, Deforming true
>   Timing: Generation 0.653 ms, Inlining 0.085 ms, Optimization 3.543 ms, Emission 8.198 ms, Total 12.478 ms
> Execution Time: 851.127 ms
> (7 rows)

Thank you for the JIT how-to !

--

I've made another go-through and got following findings:

1/ There is a whitespace right after "%prep" macro you should remove.

2/ I can see that the "docs" supackage states:
  |  Requires:	%{name}%{?_isa} = %precise_version
It the base package really necessary for the "docs" package to be installable and usable ?
If not, I'd suggest weakening the relation to the base package to Recommends or Suggests.

3/ Few days ago, the 1.6.2 version has been released
  https://github.com/citusdata/pg_auto_failover/releases/tag/v1.6.2
Please update your code to match the latest upstream release

1. Fixed trailing spaces.

2. Yes, I agree.

3. Update spec file to 1.6.2. (bug fix release) (new issues, look docs section)

Another Changes:
* Fixed cond macros for sub-pkgs builds. Now, all builds correctly.
* Description for openssl-devel added.
* Added _unpackaged_files_terminate_build (This is only activated when we build without jit extension, I would not put it in default build but for conditional, not default arguments should be fine, avoiding not packed JIT extensions)
* Switch to bcond

About man files:
I know that removing spaces is not a great solution here but this is not a problem on my side. Assuming that spaces in man files are allowed, example 'autopl example.1'
those files should be compressed by %install macro, they are not because of this:

https://github.com/rpm-software-management/rpm/blob/master/scripts/brp-compress#L24

this script won't handle files with special characters correctly.

I will try to solve it in upstream but that will take time and I need to choose workaround for now. Here are my options:
- change spaces to '-' or '_' to satisfy brp script.
- compress man files manually in spec %install macro
- try to modify sources to generate names without spaces (Not a great solution probably)
- add compression to Makefile in sources using patch file

I choose the 1st one, it seems like a good compromise until bug is fixed. Last option also looks ok-ish but it will require more changes and it is more pron to errors in the future (additional patch file).


About docs:
- Added %doc macro.
- upstream started using sphinx_rtd_theme_citus (fork of sphinx_rtd_theme). I reverted it to official version using patch file. More comments in patch file.
- Fix files wildcard (compression .gz)

SPEC: https://gist.githubusercontent.com/mkulik-rh/0899b8f07bb6fabeee0c7a94e7b8d340/raw/30a0fa9b3c890995830239808929f1296f6a0afa/pg_auto_failover.spec
Patch0: https://gist.githubusercontent.com/mkulik-rh/ace6f65a9bef641860e93bea7bbd0e78/raw/bf454e9687f5cf3d5ac9bb14d70ee06fc7abb57a/gistfile1.txt 

SRPM: https://copr-be.cloud.fedoraproject.org/results/mkulik/pg_auto_failover/srpm-builds/02742729/pg_auto_failover-1.6.2-1.fc35.src.rpm

https://copr.fedorainfracloud.org/coprs/mkulik/pg_auto_failover/builds/

Looks good, thanks for the justifications !

---

I won't accept this piece of code:
|  # To allow error-less build without packing llvmjit
|  %if %{without llvmjit}
|  %define _unpackaged_files_terminate_build 0
|  %endif

If possible, please avoid modifying the natural build process.

When you have a known list of unpacked files, remove them in in %install phase instead. That way you will still be informed, when a new unexpected file become unpacked to !
When using 'rm', do not use '-f'; for the same reason. As long as you can precisely list all file to be affected by the command, let the command stop you with it's natural behaviour (e.g. "hey, I should have removed a file but it is not there after the last rebase, what should I do?")
The very same idea is the reason why the Guidelines prohibits concealing shared libraries version number with a globs / wildcards. So the natural build process would let you know in the future, that something needs your attention.
Please keep this idea in mind during your maintainer life.

---

Issue found by fedora-review tool:


1/
If your application is a C or C++ application you must list a
BuildRequires against gcc, gcc-c++ or clang.
Note: No gcc, gcc-c++ or clang found in BuildRequires
See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/


2/
License file installed when any subpackage combination is installed.
  We overlooked this one in the last iteration. The 'docs' subpackage>
    Requires:	%{name}%{?_isa} = %precise_version
  was taking care of this.
  Either put it back, or move the license file to a standalone subpackage which will be required by all other subpackages.


3/
Package requires other packages for directories it uses.
  Note: No known owner of /usr/lib64/pgsql/bitcode/pgautofailover
Package must own all directories that it creates.
  Note: Directories without known owners:
  /usr/lib64/pgsql/bitcode/pgautofailover, /usr/lib64/pgsql/bitcode

Changelog:
* removing JIT extensions folder in %install macro instead of ignoring errors
* /usr/lib64/pgsql/bitcode/pgautofailover is now own by jit sub-pkg, /usr/lib64/pgsql/bitcode should be own by postgresql-llvmjit
* Changed back Suggest to Requires main pkg for docs sub-pkg
* Compiler added

SPEC: https://gist.githubusercontent.com/mkulik-rh/0899b8f07bb6fabeee0c7a94e7b8d340/raw/058606939caf270e8cb0a1005000ac22f77d9d05/pg_auto_failover.spec
SRPM: https://copr-be.cloud.fedoraproject.org/results/mkulik/pg_auto_failover/srpm-builds/02768450/pg_auto_failover-1.6.2-1.fc35.src.rpm

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.
[-]: Development (unversioned) .so files in -devel subpackage, if present.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[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.
[x]: License file installed when any subpackage combination is installed.
[x]: Package must own all directories that it creates.
[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.
[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: There are rpmlint messages (see attachment).
[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 does not own files or directories owned by other packages.
[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 must not depend on deprecated() packages.
[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).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[?]: Packages should try to preserve timestamps of original installed
     files.
[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]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[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: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[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: pg_auto_failover-1.6.2-1.fc36.x86_64.rpm
          pg_auto_failover-docs-1.6.2-1.fc36.noarch.rpm
          pg_auto_failover-llvmjit-1.6.2-1.fc36.x86_64.rpm
          pg_auto_failover-debuginfo-1.6.2-1.fc36.x86_64.rpm
          pg_auto_failover-debugsource-1.6.2-1.fc36.x86_64.rpm
          pg_auto_failover-1.6.2-1.fc36.src.rpm
pg_auto_failover.x86_64: W: spelling-error Summary(en_US) Postgres -> Postures, Postgraduates
pg_auto_failover.x86_64: W: spelling-error Summary(en_US) failover -> fail over, fail-over, spillover
pg_auto_failover.x86_64: W: spelling-error %description -l en_US failover -> fail over, fail-over, spillover
pg_auto_failover.x86_64: W: spelling-error %description -l en_US autoctl -> autoclave
pg_auto_failover-docs.noarch: W: spelling-error Summary(en_US) failover -> fail over, fail-over, spillover
pg_auto_failover-docs.noarch: W: spelling-error %description -l en_US failover -> fail over, fail-over, spillover
pg_auto_failover-llvmjit.x86_64: W: spelling-error Summary(en_US) failover -> fail over, fail-over, spillover
pg_auto_failover-llvmjit.x86_64: W: spelling-error %description -l en_US failover -> fail over, fail-over, spillover
pg_auto_failover-llvmjit.x86_64: W: only-non-binary-in-usr-lib
pg_auto_failover-llvmjit.x86_64: W: no-documentation
pg_auto_failover-debuginfo.x86_64: W: spelling-error Summary(en_US) failover -> fail over, fail-over, spillover
pg_auto_failover-debuginfo.x86_64: W: spelling-error %description -l en_US failover -> fail over, fail-over, spillover
pg_auto_failover-debugsource.x86_64: W: spelling-error Summary(en_US) failover -> fail over, fail-over, spillover
pg_auto_failover-debugsource.x86_64: W: spelling-error %description -l en_US failover -> fail over, fail-over, spillover
pg_auto_failover.src: W: spelling-error Summary(en_US) Postgres -> Postures, Postgraduates
pg_auto_failover.src: W: spelling-error Summary(en_US) failover -> fail over, fail-over, spillover
pg_auto_failover.src: W: spelling-error %description -l en_US failover -> fail over, fail-over, spillover
pg_auto_failover.src: W: spelling-error %description -l en_US autoctl -> autoclave
6 packages and 0 specfiles checked; 0 errors, 18 warnings.




Rpmlint (debuginfo)
-------------------
Checking: pg_auto_failover-debuginfo-1.6.2-1.fc36.x86_64.rpm
pg_auto_failover-debuginfo.x86_64: W: spelling-error Summary(en_US) failover -> fail over, fail-over, spillover
pg_auto_failover-debuginfo.x86_64: W: spelling-error %description -l en_US failover -> fail over, fail-over, spillover
1 packages and 0 specfiles checked; 0 errors, 2 warnings.





Unversioned so-files
--------------------
pg_auto_failover: /usr/lib64/pgsql/pgautofailover.so

Source checksums
----------------
https://github.com/citusdata/pg_auto_failover/archive/refs/tags/v1.6.2.tar.gz :
  CHECKSUM(SHA256) this package     : 8af35ac18e39b9b016c5410b317bdd81a3351f0d6f65bb77484d14936d17bb74
  CHECKSUM(SHA256) upstream package : 8af35ac18e39b9b016c5410b317bdd81a3351f0d6f65bb77484d14936d17bb74


Requires
--------
pg_auto_failover (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libm.so.6()(64bit)
    libpq.so.private13-5()(64bit)
    postgresql-contrib
    postgresql-server
    rtld(GNU_HASH)

pg_auto_failover-docs (rpmlib, GLIBC filtered):
    pg_auto_failover(x86-64)

pg_auto_failover-llvmjit (rpmlib, GLIBC filtered):
    pg_auto_failover(x86-64)
    postgresql-llvmjit

pg_auto_failover-debuginfo (rpmlib, GLIBC filtered):

pg_auto_failover-debugsource (rpmlib, GLIBC filtered):



Provides
--------
pg_auto_failover:
    pg_auto_failover
    pg_auto_failover(x86-64)

pg_auto_failover-docs:
    pg_auto_failover-docs

pg_auto_failover-llvmjit:
    pg_auto_failover-llvmjit
    pg_auto_failover-llvmjit(x86-64)

pg_auto_failover-debuginfo:
    debuginfo(build-id)
    pg_auto_failover-debuginfo
    pg_auto_failover-debuginfo(x86-64)

pg_auto_failover-debugsource:
    pg_auto_failover-debugsource
    pg_auto_failover-debugsource(x86-64)



Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
Command line :/usr/bin/fedora-review -n pg_auto_failover
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, Generic, C/C++
Disabled plugins: Ocaml, fonts, Haskell, PHP, SugarActivity, Python, Java, Perl, R
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

==================================================================================================

Revieved by: Michal Schorm

Reviewer notes:

* The '/usr/lib64/pgsql/pgautofailover.so' file is a plugin for PostgreSQL database.
  It is not versioned nor is expected to be.
* The author provided *.rpmlintrc file to deal with the RPMLint spelling errors that are false positives or otherwise wrong.

I _ACCEPT_ this package submission.
Marek, feel free to contact me anytime, should you need any additional guidance or just a help in the Fedora import/build/update process.

That was an in-depth review. I appreciate it. Thank you.

One nitpick:

> %if 0%{?fedora} < 35

This can cause problems when downstreaming. E.g. This becomes true on RH 9 (or 10). Because the macro fedora is not defined there and it becomes 0 < 35 which is True.
It is better to write it as:

> %if 0%{?fedora} && 0%{?fedora} < 35

or you can reverse the condition

%if 0%{?fedora} < 35
   #nothing to do here because.....
%elif
  Requires:.....
%endif

But this definitely does not block the review.
I explained to Marek the processes in Fedora and how to do the things.
I granted Marek the packager role.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/pg_auto_failover