[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

   What|Removed |Added

 Status|POST|CLOSED
 Resolution|--- |CURRENTRELEASE
Last Closed||2019-09-17 15:43:06



-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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/package-review@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #21 from Zbigniew Jędrzejewski-Szmek  ---
I see this has been built, but no bodhi update for F27?

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #20 from Gwyn Ciesla  ---
(fedrepo-req-admin):  The Pagure repository was created at
https://src.fedoraproject.org/rpms/python-anyconfig. You may commit to the
branch "f27" in about 10 minutes.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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

Miro Hrončok  changed:

   What|Removed |Added

 CC||mhron...@redhat.com



--- Comment #19 from Miro Hrončok  ---
Some notes here:

%if 0%{?rhel} == 7
BuildRequires:  python-devel
BuildRequires:  python-setuptools
%else
BuildRequires:  python2-devel
BuildRequires:  python2-setuptools
%endif
...
%if 0%{?rhel} == 7
Requires:   python-setuptools
%else
Requires:   python2-setuptools
%endif

Both python2-devel and python2-setuptools is available (provided) in RHEL 7.



%if 0%{?rhel} == 7
%{python_sitelib}/*
%else
%{python2_sitelib}/*
%endif

%{python2_sitelib} is available in EPEL 7 (if you are targeting plain RHEL,
feel free to ignore this).

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

   What|Removed |Added

 Status|ASSIGNED|POST
  Flags||fedora-review+



--- Comment #18 from Zbigniew Jędrzejewski-Szmek  ---
Note: I bumped the version to 0.9.4 manually, the spec file still says 0.9.3.

The %changelog in the rpm package is _not_ supposed to be a reflection of the
vcs logs of upstream. It should be a short overview of high-level changes
relevant to the user of the package. Hence, it'd even be better to say "0.9.4
Update to latest upstream version, minor cleanups", i.e. give no details,
rather than overwhelm the users with details they don't care about. E.g. there
is no python3.3 in any supported Fedora, and some deprecated or replaced
methods are not important for the user. See
https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs. This is a minor
thing, also a bit of personal preference, and I'm not saying you have to change
the style, and especially not for past entries, but please consider it for the
future.

+ package name is OK
+ license is acceptable for Fedora (MIT)
+ license is specified correctly
+ builds and install OK
+ Provides/Requires/BR look correct
+ new python packaging macros are used
+ %python_provide macros is used

Package is APPROVED.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #17 from Brett Lentz  ---
New spec and srpm are now up at the location in comment #7.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #16 from Zbigniew Jędrzejewski-Szmek  ---
OK.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #15 from Satoru SATOH  ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #10)
> And also (a question for upstream): why is the executable called
> "anyconfig_cli" and not just "anyconfig"?

"anyconfig" feels too generic for me and it's just a demo program of this
library at the time I first made it.
And, to change its name causes other issues (I have to update all its
references in the doc at least),
so I want to keep as it is.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #14 from Zbigniew Jędrzejewski-Szmek  ---
Is there a new version of the spec file for review?

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #13 from Satoru SATOH  ---
FYI.

I've released the new version 0.9.4 contains rpm related fixes and ...

(In reply to Zbigniew Jędrzejewski-Szmek from comment #12)
> (In reply to Brett Lentz from comment #11)
> > (In reply to Zbigniew Jędrzejewski-Szmek from comment #10)
> > > > %global sum Python library to load and dump configuration files in 
> > > > various formats
> > > Using normal Summary: and then %summary subsequently saves one line ;)
> > > 
> > 
> > Not true. The summary is used in 3 places because of the python2 & python3
> > sub-packaged. The macro saves copy/pasting the same text in 3 places.  :)
> 
> I didn't mean copying the text three times. I meant something like this:
> Summary: blah blah blah
> ...
> Summary: %summary
> ...
> Summary: %summary
> 
> Macro %summary is automatically defined to the contents of the last Summary
> line.

I didn't know this works. Thanks a lot for letting me know. Fixed it in the
upstream.

> > > During build I see the following error:
> > > > import cbor
> > > > ImportError: No module named cbor
> > > Is some dependency missing?

To keep dependencies at a minimum, anyconfig can process most ImportError-es at
runtime correctly and works well w/o some dependencies like cbor are missing.
And the new version becomes dependent on only some popular libraries by
default, so this kind of error disappears as much as possible, I think.

> > > and later:
> > > > toml.py:docstring of 
> > > > anyconfig.backend.toml.Parser._load_from_stream_fn:6: WARNING: 
> > > > Definition list ends without a blank line; unexpected unindent.
> > > > /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/backend/xml.py:docstring
> > > >  of anyconfig.backend.xml._tweak_ns:4: WARNING: Field list ends without 
> > > > a blank line; unexpected unindent.
> > > > /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/docs/api/anyconfig.cli.rst:4:
> > > >  WARNING: autodoc: failed to import module u'anyconfig.cli'; the 
> > > > following exception was raised:
> > > > Traceback (most recent call last):
> > > >   File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 
> > > > 658, in import_object
> > > > __import__(self.modname)
> > > >   File 
> > > > "/builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/cli.py",
> > > >  line 42, in 
> > > > sys.stdout = codecs.getwriter(_ENCODING)(sys.stdout)
> > > >   File "/usr/lib64/python2.7/codecs.py", line 1009, in getwriter
> > > > return lookup(encoding).streamwriter
> > > > TypeError: lookup() argument 1 must be string, not None
> > > > done
> > > 
> > This looks like a bug in the docs. I'll point it out to upstream.
> Great.

Still I've been looking into this and may take some time to fix them
unfortunately.
But I don't think it's critical for packaging.

> > > And now the hard part: what is the difference in behaviour or output 
> > > between
> > > anyconfig-2 and anyconfig-3?
> > 
> > There is no difference, AFAICS.
> OK. If there is no difference, then only one version of the executable
> should be packed. See
> https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.2Fbin:
> > If the executables provide the same functionality independent of whether 
> > they are run on top of Python 2 or Python 3, then only the Python 3 version 
> > of the executable should be packaged.
> 
> You went through the effort to get the symlinks right, but it now seems that
> not actually necessary ;(

Fixed in the upstream.


Brett-san,
could you please take a look at the RPM SPEC template in the upstream and try
to arrange the new version of RPM SPEC and srpm?
I tried to keep there are least differences between mine (upstream) and yours
as much as possible but maybe there are issues remained I'm not aware of or
forgot.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #12 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Brett Lentz from comment #11)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #10)
> > > %global sum Python library to load and dump configuration files in 
> > > various formats
> > Using normal Summary: and then %summary subsequently saves one line ;)
> > 
> 
> Not true. The summary is used in 3 places because of the python2 & python3
> sub-packaged. The macro saves copy/pasting the same text in 3 places.  :)

I didn't mean copying the text three times. I meant something like this:
Summary: blah blah blah
...
Summary: %summary
...
Summary: %summary

Macro %summary is automatically defined to the contents of the last Summary
line.

> > During build I see the following error:
> > > import cbor
> > > ImportError: No module named cbor
> > Is some dependency missing?
> > 
> 
> There is support for a backend (cbor) that does not currently have a package
> in Fedora. If it's okay with you, I'd prefer to not block this package on
> the one missing backend.
> 
> This anyconfig package is a dependency of the package I'm ultimately looking
> to get into Fedora (molecule). However, anyconfig's support for cbor is not
> a part of my critical path.
> 
> I am willing to work on a python-cbor package after anyconfig is in Fedora,
> if that works for you.

Sure, whatever works. There is no obligation to package everything possible. If
something is an optional dependency, it's entirely reasonable to leave it for
later or to somebody else who actually needs it.

> > and later:
> > > toml.py:docstring of 
> > > anyconfig.backend.toml.Parser._load_from_stream_fn:6: WARNING: Definition 
> > > list ends without a blank line; unexpected unindent.
> > > /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/backend/xml.py:docstring
> > >  of anyconfig.backend.xml._tweak_ns:4: WARNING: Field list ends without a 
> > > blank line; unexpected unindent.
> > > /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/docs/api/anyconfig.cli.rst:4:
> > >  WARNING: autodoc: failed to import module u'anyconfig.cli'; the 
> > > following exception was raised:
> > > Traceback (most recent call last):
> > >   File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 
> > > 658, in import_object
> > > __import__(self.modname)
> > >   File 
> > > "/builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/cli.py", 
> > > line 42, in 
> > > sys.stdout = codecs.getwriter(_ENCODING)(sys.stdout)
> > >   File "/usr/lib64/python2.7/codecs.py", line 1009, in getwriter
> > > return lookup(encoding).streamwriter
> > > TypeError: lookup() argument 1 must be string, not None
> > > done
> > 
> This looks like a bug in the docs. I'll point it out to upstream.
Great.

> > And now the hard part: what is the difference in behaviour or output between
> > anyconfig-2 and anyconfig-3?
> 
> There is no difference, AFAICS.
OK. If there is no difference, then only one version of the executable should
be packed. See
https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.2Fbin:
> If the executables provide the same functionality independent of whether they 
> are run on top of Python 2 or Python 3, then only the Python 3 version of the 
> executable should be packaged.

You went through the effort to get the symlinks right, but it now seems that
not actually necessary ;(

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #11 from Brett Lentz  ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #10)
> I'll take the review.
> 

Great! Thanks!

> > %global sum Python library to load and dump configuration files in various 
> > formats
> Using normal Summary: and then %summary subsequently saves one line ;)
> 

Not true. The summary is used in 3 places because of the python2 & python3
sub-packaged. The macro saves copy/pasting the same text in 3 places.  :)


> > %global debug_package %{nil}
> That looks suspicious. Why do you need this?
> Package builds fine without it.

Removed.

> 
> > %{__rm}
> Eh, using a macro here is entirely pointless. It just makes the commands
> harder to read (and longer). The guidelines say that macros should be used
> for some *directories*
> [https://fedoraproject.org/wiki/Packaging:Guidelines#Macros], but even that
> makes little sense nowadays.
> 

Fixed.

> > https://github.com/ssato/%{name}
> I know people love macros, but this makes it impossible to just click on
> this and open it in a browser… It's a matter of preference, but I don't see
> the advantage of using a macro here.
> 

Fixed.

> > Source0:%{url}/archive/RELEASE-%{version}.tar.gz
> This should be ...RELEASE_{%version}...
> 

Fixed.

> > %defattr(-,root,root,-)
> Not necessary in Fedora and somewhat recent RHEL.
> 

Fixed

> - Large documentation must go in a -doc subpackage. Large could be size
>   (~1MB) or number of files.
>   Note: Documentation size is 2662400 bytes in 126 files.
>   See:
>   http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation
> 
> It'd be nice to split out a python-anyconfig-doc subpackage with the docs.
> 

Fixed.

> 
> During build I see the following error:
> > import cbor
> > ImportError: No module named cbor
> Is some dependency missing?
> 

There is support for a backend (cbor) that does not currently have a package in
Fedora. If it's okay with you, I'd prefer to not block this package on the one
missing backend.

This anyconfig package is a dependency of the package I'm ultimately looking to
get into Fedora (molecule). However, anyconfig's support for cbor is not a part
of my critical path.

I am willing to work on a python-cbor package after anyconfig is in Fedora, if
that works for you.


> and later:
> > toml.py:docstring of anyconfig.backend.toml.Parser._load_from_stream_fn:6: 
> > WARNING: Definition list ends without a blank line; unexpected unindent.
> > /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/backend/xml.py:docstring
> >  of anyconfig.backend.xml._tweak_ns:4: WARNING: Field list ends without a 
> > blank line; unexpected unindent.
> > /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/docs/api/anyconfig.cli.rst:4:
> >  WARNING: autodoc: failed to import module u'anyconfig.cli'; the following 
> > exception was raised:
> > Traceback (most recent call last):
> >   File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 658, 
> > in import_object
> > __import__(self.modname)
> >   File 
> > "/builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/cli.py", 
> > line 42, in 
> > sys.stdout = codecs.getwriter(_ENCODING)(sys.stdout)
> >   File "/usr/lib64/python2.7/codecs.py", line 1009, in getwriter
> > return lookup(encoding).streamwriter
> > TypeError: lookup() argument 1 must be string, not None
> > done
> 


This looks like a bug in the docs. I'll point it out to upstream.


> And now the hard part: what is the difference in behaviour or output between
> anyconfig-2 and anyconfig-3?

There is no difference, AFAICS.

> 
> And also (a question for upstream): why is the executable called
> "anyconfig_cli" and not just "anyconfig"?

Fixed in the spec until it's fixed upstream.


I've updated the spec and srpm. Same URLs as in comment #7.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 CC||zbys...@in.waw.pl
   Assignee|nob...@fedoraproject.org|zbys...@in.waw.pl



--- Comment #10 from Zbigniew Jędrzejewski-Szmek  ---
I'll take the review.

> %global sum Python library to load and dump configuration files in various 
> formats
Using normal Summary: and then %summary subsequently saves one line ;)

> %global debug_package %{nil}
That looks suspicious. Why do you need this?
Package builds fine without it.

> %{__rm}
Eh, using a macro here is entirely pointless. It just makes the commands harder
to read (and longer). The guidelines say that macros should be used for some
*directories* [https://fedoraproject.org/wiki/Packaging:Guidelines#Macros], but
even that makes little sense nowadays.

> https://github.com/ssato/%{name}
I know people love macros, but this makes it impossible to just click on this
and open it in a browser… It's a matter of preference, but I don't see the
advantage of using a macro here.

> Source0:%{url}/archive/RELEASE-%{version}.tar.gz
This should be ...RELEASE_{%version}...

> %defattr(-,root,root,-)
Not necessary in Fedora and somewhat recent RHEL.

- Large documentation must go in a -doc subpackage. Large could be size
  (~1MB) or number of files.
  Note: Documentation size is 2662400 bytes in 126 files.
  See:
  http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation

It'd be nice to split out a python-anyconfig-doc subpackage with the docs.


During build I see the following error:
> import cbor
> ImportError: No module named cbor
Is some dependency missing?

and later:
> toml.py:docstring of anyconfig.backend.toml.Parser._load_from_stream_fn:6: 
> WARNING: Definition list ends without a blank line; unexpected unindent.
> /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/backend/xml.py:docstring
>  of anyconfig.backend.xml._tweak_ns:4: WARNING: Field list ends without a 
> blank line; unexpected unindent.
> /builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/docs/api/anyconfig.cli.rst:4:
>  WARNING: autodoc: failed to import module u'anyconfig.cli'; the following 
> exception was raised:
> Traceback (most recent call last):
>   File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 658, in 
> import_object
> __import__(self.modname)
>   File 
> "/builddir/build/BUILD/python-anyconfig-RELEASE_0.9.3/anyconfig/cli.py", line 
> 42, in 
> sys.stdout = codecs.getwriter(_ENCODING)(sys.stdout)
>   File "/usr/lib64/python2.7/codecs.py", line 1009, in getwriter
> return lookup(encoding).streamwriter
> TypeError: lookup() argument 1 must be string, not None
> done

And now the hard part: what is the difference in behaviour or output between
anyconfig-2 and anyconfig-3?

And also (a question for upstream): why is the executable called
"anyconfig_cli" and not just "anyconfig"?

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #9 from Brett Lentz  ---
I've fixed the url and the buildrequires. File URLs haven't changed since
comment #7

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #8 from Troy Curtis  ---
The source url, which expands to
https://github.com/ssato/python-anyconfig/archive/0.9.3/anyconfig-0.9.3.tar.gz
gives a "File not Found" error.

BuildRequires actually belong to the source rpm, not individual subpackages, so
I'd suggest moving them up a level.  This will also make clear the 'make' is a
duplicate dependency.

For some reason my `fedora-review` invocation is having some trouble, so these
two items will have to suffice for the moment.  I didn't notice other issues
though, and as I'm not (yet) a packager, I can't take it all the way to
approved anyhow.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #7 from Brett Lentz  ---
Updated spec:
https://wakko666.fedorapeople.org/python-anyconfig/python-anyconfig.spec

Updated srpm:
https://wakko666.fedorapeople.org/python-anyconfig/python-anyconfig-0.9.3-1.fc28.src.rpm

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #6 from Satoru SATOH  ---
Troy-san, thanks a lot for your comments!

(In reply to Troy Curtis from comment #3)
> The version suffixes are for the corresponding python version.
> 
> %{python2_version} and %{python3_version} already have the minor version,
> for instance 2.7, and 3.6.  So the move would be:
> 
> mv %{buildroot}%{_bindir}/anyconfig_cli
> %{buildroot}%{_bindir}/anyconfig_cli-%{python3_version}

I guess that I made so already by the previous commits I mentioned:

ssato@localhost% rpm -ql python2-anyconfig | grep anyconfig_cli |
LC_ALL=en_US.UTF-8 xargs ls -l
lrwxrwxrwx. 1 root root   17 Feb  1 23:04 /usr/bin/anyconfig_cli ->
anyconfig_cli-2.7
lrwxrwxrwx. 1 root root   17 Feb  1 23:04 /usr/bin/anyconfig_cli-2 ->
anyconfig_cli-2.7
-rwxr-xr-x. 1 root root  400 Feb  1 23:04 /usr/bin/anyconfig_cli-2.7
lrwxrwxrwx. 1 root root   22 Feb  1 23:04
/usr/share/man/man1/anyconfig_cli.1.gz -> anyconfig_cli-2.7.1.gz
lrwxrwxrwx. 1 root root   22 Feb  1 23:04
/usr/share/man/man1/anyconfig_cli-2.1.gz -> anyconfig_cli-2.7.1.gz
-rw-r--r--. 1 root root 1724 May  3  2017
/usr/share/man/man1/anyconfig_cli-2.7.1.gz
ssato@localhost% rpm -ql python3-anyconfig | grep anyconfig_cli |
LC_ALL=en_US.UTF-8 xargs ls -l
lrwxrwxrwx. 1 root root   17 Feb  1 23:04 /usr/bin/anyconfig_cli-3 ->
anyconfig_cli-3.6
-rwxr-xr-x. 1 root root  400 Feb  1 23:04 /usr/bin/anyconfig_cli-3.6
lrwxrwxrwx. 1 root root   22 Feb  1 23:04
/usr/share/man/man1/anyconfig_cli-3.1.gz -> anyconfig_cli-3.6.1.gz
-rw-r--r--. 1 root root 1724 May  3  2017
/usr/share/man/man1/anyconfig_cli-3.6.1.gz
ssato@localhost%

> And for the python2 subpackage, don't copy the executable for the
> unversioned executable, just add a second symlink:
> 
> ln -s anyconfig_cli-%{python3_version} %{buildroot}%{_bindir}/anyconfig_cli

I've fixed it:
https://github.com/ssato/python-anyconfig/commit/51a7e4fa1376e9daa1b90ea0be861bbf6f4b325d#diff-49f0020084413dbd6fb3815a74500f70

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #5 from Satoru SATOH  ---
(In reply to Troy Curtis from comment #4)
> Also the Group: tag is not used in Fedora.

Thanks for you comment. I removed it in the upstream:
https://github.com/ssato/python-anyconfig/commit/d45903b65ebe8e2a653094f3956dd9d32079c441#diff-49f0020084413dbd6fb3815a74500f70

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #4 from Troy Curtis  ---
Also the Group: tag is not used in Fedora.

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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



--- Comment #3 from Troy Curtis  ---
License file change looks good.

The version suffixes are for the corresponding python version.

%{python2_version} and %{python3_version} already have the minor version, for
instance 2.7, and 3.6.  So the move would be:

mv %{buildroot}%{_bindir}/anyconfig_cli
%{buildroot}%{_bindir}/anyconfig_cli-%{python3_version}

Then symlink from the major version only variant:

ln -s anyconfig_cli-%{python3_version} %{buildroot}%{_bindir}/anyconfig_cli-3

And for the python2 subpackage, don't copy the executable for the unversioned
executable, just add a second symlink:

ln -s anyconfig_cli-%{python3_version} %{buildroot}%{_bindir}/anyconfig_cli

And then mirror this for the man pages.  

You can take a look at the python2 and python3 packages to get the idea (though
it doesn't use a '-', but for this package you should).

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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

Satoru SATOH  changed:

   What|Removed |Added

 CC||ss...@redhat.com



--- Comment #2 from Satoru SATOH  ---
FYI.

I'm not sure how to fix the issues in a correct manner but made some changes in
its upstream:

> Since executables are being shipped in both python 2 and 3 subpackages, each
> package needs to have executables with -X and -X.Y version suffixes [0], with
> python2 providing an unversioned symlink executable.

https://github.com/ssato/python-anyconfig/commit/98d28d056299abf2f83feffbc14ec4c157757214#diff-49f0020084413dbd6fb3815a74500f70

> The LICENSE.MIT file needs to be included with %license.

https://github.com/ssato/python-anyconfig/commit/be544779e697674520def3907cf090a42c0f715f#diff-49f0020084413dbd6fb3815a74500f70

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1538658] Review Request: python-anyconfig - common API to load and dump configuration files

[bugzilla]

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

Troy Curtis  changed:

   What|Removed |Added

 CC||troycurti...@gmail.com



--- Comment #1 from Troy Curtis  ---
Since executables are being shipped in both python 2 and 3 subpackages, each
package needs to have executables with -X and -X.Y version suffixes [0], with
python2 providing an unversioned symlink executable.

The executables exists in both subpackages, but only the python2 package has
the man page.

The LICENSE.MIT file needs to be included with %license.

0: https://fedoraproject.org/wiki/Packaging:Python#Naming

-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org