PR#4: Spec cleanup (also fixes a missing dependency on six) - rpms/python-lesscpy

		`@@ -0,0 +1,24 @@`
		`+ From 964905dc09406a55097d9c644b45bbec6944170a Mon Sep 17 00:00:00 2001`
		`+ From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= <miro@hroncok.cz>`
		`+ Date: Tue, 12 Jun 2018 13:43:47 +0200`
		`+ Subject: [PATCH] Allow to run tests from versioned folder`
		`+`
		`+ If I download and unpack lesscpy-0.13.0 the tests from within`
		`+ were failing, because the 0.13.0 got split. this fixes it.`
		`+ ---`
		`+ test/core.py \| 2 +-`
		`+ 1 file changed, 1 insertion(+), 1 deletion(-)`
		`+`
		`+ diff --git a/test/core.py b/test/core.py`
		`+ index 438ec54..63e3689 100644`
		`+ --- a/test/core.py`
		`+ +++ b/test/core.py`
		`+ @@ -24,7 +24,7 @@ def find_and_load_cases(cls, less_dir, css_dir, less_files=None, css_minimized=T`
		`+ else:`
		`+ LESS = glob.glob(os.path.join(_less_path, '*.less'))`
		`+ for less in LESS:`
		`+ - lessf = less.split('.')[0].split('/')[-1]`
		`+ + lessf = less.rpartition('.')[0].split('/')[-1]`
		`+ css = os.path.join(_css_path, lessf + '.css')`
		`+ if css_minimized:`
		`+ mincss = os.path.join(_css_path, lessf + '.min.css')`

python-lesscpy-fix-setup-encoding-issue.patch

file removed

-12

		`@@ -1,12 +0,0 @@`
		`- diff -up python3-lesscpy-0.9j-1.fc19/setup.py.bak python3-lesscpy-0.9j-1.fc19/setup.py`
		`- --- python3-lesscpy-0.9j-1.fc19/setup.py.bak 2013-08-21 10:41:30.341289892 +0200`
		`- +++ python3-lesscpy-0.9j-1.fc19/setup.py 2013-08-21 10:42:25.308195797 +0200`
		`- @@ -24,6 +24,6 @@ setup(`
		`- 'lesscpy/test/css/issues/*.css',`
		`- 'lesscpy/test/less/*.less',`
		`- 'lesscpy/test/less/issues/*.less',]},`
		`- - license=open('LICENSE').read(),`
		`- - long_description=open('README.rst').read(),`
		`- + license=open('LICENSE', encoding='utf-8').read(),`
		`- + long_description=open('README.rst', encoding='utf-8').read(),`
		`- )`

python-lesscpy.spec

file modified

+52 -54

		`@@ -1,28 +1,25 @@`
		`- %if 0%{?fedora} > 12`
		`- %global with_python3 1`
		`+ %global pypi_name lesscpy`
		`+`
		`+ %if 0%{?rhel} > 7`
		`+ # Disable python2 build by default`
		`+ %bcond_with python2`
		`%else`
		`- %{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print (get_python_lib())")}`
		`+ %bcond_without python2`
		`%endif`

		`- %global pypi_name lesscpy`
		`-`
		`Name: python-%{pypi_name}`
		`Version: 0.13.0`
		`- Release: 2%{?dist}`
		`+ Release: 3%{?dist}`
		`Summary: Lesscss compiler`

		`License: MIT`
		`URL: https://github.com/robotis/lesscpy`
		`Source0: https://pypi.python.org/packages/source/l/%{pypi_name}/%{pypi_name}-%{version}.tar.gz`
		`+`
		`+ Patch0: https://github.com/lesscpy/lesscpy/pull/99.patch#/%{pypi_name}-fix-test.patch`
		`+`
		`BuildArch: noarch`

		`- BuildRequires: python2-devel`
		`- BuildRequires: python2-setuptools`
		`- BuildRequires: python2-ply`
		`- BuildRequires: python2-nose`
		`- BuildRequires: python2-coverage`
		`- BuildRequires: python2-flake8`
		`-`
		`%global _description\`
		`A compiler written in python 3 for the lesscss language. For those of us not\`
		`willing/able to have node.js installed in our environment. Not all features\`
		`@@ -31,90 +28,91 @@`

		`%description %_description`

		`+ %if %{with python2}`
		`%package -n python2-lesscpy`
		`- Summary: %summary`
		`+ Summary: %summary`
		`Requires: python2-ply`
		`+ Requires: python2-six`
		`+ BuildRequires: python2-devel`
		`+ BuildRequires: python2-setuptools`
		`+ BuildRequires: python2-ply`
		`+ BuildRequires: python2-nose`
		`+ BuildRequires: python2-coverage`
		`+ BuildRequires: python2-flake8`
		`+ BuildRequires: python2-six`
		`%{?python_provide:%python_provide python2-lesscpy}`

		`%description -n python2-lesscpy %_description`
		`+ %endif # with python2`

		`- %if 0%{?with_python3}`
		`%package -n python3-lesscpy`
		`- Summary: Lesscss compiler`
		`+ Summary: %summary`
		`Requires: python3-ply`
		`+ Requires: python3-six`
		`BuildRequires: python3-devel`
		`BuildRequires: python3-setuptools`
		`BuildRequires: python3-ply`
		`BuildRequires: python3-nose`
		`BuildRequires: python3-flake8`
		`BuildRequires: python3-coverage`
		`+ BuildRequires: python3-six`
		`+ %{?python_provide:%python_provide python3-lesscpy}`
		`+`
		`%description -n python3-lesscpy`
		`A compiler written in python 3 for the lesscss language. For those of us not`
		`willing/able to have node.js installed in our environment. Not all features`
		`of lesscss are supported (yet). Some features wil probably never be`
		`supported (JavaScript evaluation).`
		`- %endif #with_python3`

		`%prep`
		`- %setup -q -n %{pypi_name}-%{version}`
		`-`
		`- %if 0%{?with_python3}`
		`- rm -rf %{py3dir}`
		`- cp -a . %{py3dir}`
		`- #fix utf8 encoding issue occurring only under py3`
		`- pushd %{py3dir}`
		`- find %{py3dir} -name '*.py' \| xargs sed -i '1s\|^#!/usr/bin/python\|#!%{__python3}\|'`
		`- popd`
		`- %endif`
		`+ %autosetup -p1 -n %{pypi_name}-%{version}`

		`%build`
		`- %{__python} setup.py build`
		`- %if 0%{?with_python3}`
		`- pushd %{py3dir}`
		`- export LANG=en_US.utf8`
		`- env`
		`- %{__python3} setup.py build`
		`- popd`
		`- %endif # with_python3`
		`+ %if %{with python2}`
		`+ %py2_build`
		`+ %endif # with python2`

		`+ %py3_build`

		`- %install`
		`- %{__python} setup.py install --skip-build --root %{buildroot}`

		`- %if 0%{?with_python3}`
		`+ %install`
		`+ %if %{with python2}`
		`+ %py2_install`
		`rm %{buildroot}/%{_bindir}/lesscpy`
		`- pushd %{py3dir}`
		`- %{__python3} setup.py install --skip-build --root %{buildroot}`
		`+ %endif # with python2`

		`+ %py3_install`
		`# link for backwards compatibility. consider removal in Fedora 30+`
		`ln -s ./lesscpy %{buildroot}/%{_bindir}/py3-lesscpy`
		`- popd`
		`- %endif # with_python3`


		`- #%check`
		`- #nosetests -v`
		`- #%if 0%{?with_python3}`
		`- #pushd %{py3dir}`
		`- #nosetests-3.3 -v`
		`- #popd`
		`- #%endif # with_python3`
		`+ %check`
		`+ %if %{with python2}`
		`+ %{__python2} -m nose -v`
		`+ %endif # with python2`
		`+ %{__python3} -m nose -v`

		`+ %if %{with python2}`
		`%files -n python2-lesscpy`
		`%doc LICENSE`
		`- %{python_sitelib}/%{pypi_name}`
		`- %{python_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info`
		`- %if 0%{?with_python3}`
		`+ %{python2_sitelib}/%{pypi_name}`
		`+ %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info`
		`+ %endif # with python2`
		`+`
		`%files -n python3-lesscpy`
		`%doc LICENSE`
		`+ %{_bindir}/lesscpy`
		`%{_bindir}/py3-lesscpy`
		`%{python3_sitelib}/%{pypi_name}`
		`%{python3_sitelib}/%{pypi_name}*.egg-info`
		`- %endif # with_python3`
		`- %{_bindir}/lesscpy`


		`%changelog`
		`+ * Tue Jun 12 2018 Miro Hrončok <mhroncok@redhat.com> - 0.13.0-3`
		`+ - General spec cleanup`
		`+ - Add missing dependency on six`
		`+ - Run the tests`
		`+`
		`* Tue Jun 12 2018 Miro Hrončok <mhroncok@redhat.com> - 0.13.0-2`
		`- /usr/bin/lesscpy is Python 3`

no initial comment

cstratak commented on line 23 of python-lesscpy.spec 5 years ago

Small nitpick. Could provide some more info about the patch in a comment here?

cstratak commented 5 years ago

Overall it LGTM.

cstratak commented 5 years ago

Ah also the NVR will need to be increased I guess.

churchyard commented 5 years ago

right, let me bump the release and add a changelog entry

1 new commit added

%changelog entry for cleanup

5 years ago

churchyard commented 5 years ago

added in a separate commit

churchyard commented 5 years ago

BTW the patch is a link to upstream PR with all the info. the patch also contains the commit message. I think it's ok without a comment.

Pull-Request has been merged by cstratak

5 years ago

mrunge commented 5 years ago

I would really prefer:
- the changes in a single commit, and not a series of patches
- when I'm asked to merge this, then I'd like to do it myself. This patch has not been 7 hours old! There was no urgency at all. No life would have been harmed by waiting another hour.

churchyard commented 5 years ago

Sorry about that. I've asked @cstratak for a review and it might have sounded like I'm pressing this to be merged ASAP. My fault.

As for the 1 commit thing I think it's not good approach to have one large cleanup commit because it is sometimes hard when you say: why the hell is this line here?, you check git blame and it just says cleanup. Hence I try to make the changes atomic.

mrunge commented 5 years ago

You are a proven packager; there is no need to involve another pp to merge anything.

If something is useful in other branches, I'm trying to keep the number of commits as small as possible. That makes cherry-picks from one branch to another easier.
If you want to keep it separate, please open multiple pull requests. Otherwise those 9 commits are now merged with a single "spec cleanup merge".

churchyard commented 5 years ago