[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 Stephen changed: What|Removed |Added CC||fed...@nuclearsunshine.com --- Comment #21 from Stephen --- Per https://github.com/jonleighton/gedit-trailsave/issues/10 this was ported to Python 3 three months ago - any interest in reviving this bug? -- You are receiving this mail because: You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 Guillaume Kulakowski changed: What|Removed |Added CC|package-review@lists.fedora | |project.org | --- Comment #19 from Guillaume Kulakowski --- The bug is not fixed and the author don't want to update his extension. I think it's a dead project I think we can close this bug ? -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=PUDS05EgTR&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #18 from Björn Esser --- Any news here, Guillaume? -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=YPFWl5p04M&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #17 from Guillaume Kulakowski --- Know issue: https://github.com/jonleighton/gedit-trailsave/issues/10 -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=vxZYHxGjdv&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #16 from Guillaume Kulakowski --- Hi, my initial review request was made on F18. With F19 ans Python3, this plugin doesn't work fine. I report bug upstream. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=XYvXRqVVx3&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #15 from Michael Schwendt --- Comment on attachment 761229 --> https://bugzilla.redhat.com/attachment.cgi?id=761229 improved spec-file some good findings in there -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=KUzjxSPahP&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007
--- Comment #14 from Björn Esser ---
(In reply to Guillaume Kulakowski from comment #13)
Hi Guillaume! Sorry for the little delay. :)
> some question :
> - Why modify github Source0 ? I have followed the guideline :
> http://fedoraproject.org/wiki/Packaging:SourceURL#Github
It was just meant a proposal, nothing mandatory. Both are fine with the
guideline. My proposal makes things easier when it comes to updates; you won't
have to look for new commit-sha, just adjust version-tag...
> - %global __python %{__python3}.
> OK, but with condition, because Gedit plugins on F18 use Python2.
OK. Then just add the conditional. I'm fine with that.
> - http://github.com/jonleighton/%{name}
> Is compatible with readability guideline ? OK to use variable on Source0,
> but I think that use variable on url and description harm to readability
This goes fine with readability guidelines. I have it this way in my packages
as well and nobody complained about it. I think having ONE macro in url-tag is
ok, since it still makes sense when reading it --> url ends on pkg-name.
> > Don't never-ever cloak your email-adress in spec-file...
> Why ?
Forget about it. Guidelines say it's OK. But on bugzilla your email-address is
public viewable either...
Just let me know, when I can start over with new spec/srpm...
Cheers,
Björn
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=gRezPD7LbN&a=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007
--- Comment #13 from Guillaume Kulakowski ---
Hi,
some question :
- Why modify github Source0 ? I have followed the guideline :
http://fedoraproject.org/wiki/Packaging:SourceURL#Github
- %global __python %{__python3}.
OK, but with condition, because Gedit plugins on F18 use Python2.
- http://github.com/jonleighton/%{name}
Is compatible with readability guideline ? OK to use variable on Source0, but I
think that use variable on url and description harm to readability
> Don't never-ever cloak your email-adress in spec-file...
Why ?
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=EV90oKYDTK&a=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #12 from Germán Racca --- (In reply to Björn Esser from comment #11) > Not really, so I looked around the web and found this page: > https://live.gnome.org/Gedit/PythonPluginHowTo > > So Germán was horribly wrong... That "horribly" didn't sound really good :( Anyway, thanks for the info. I'm about to package another gedit plugin and will take advantage of any discussion that happens here :) -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=hU9iuLid9v&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007
--- Comment #11 from Björn Esser ---
Created attachment 761229
--> https://bugzilla.redhat.com/attachment.cgi?id=761229&action=edit
improved spec-file
(In reply to Guillaume Kulakowski from comment #10)
> I look other Gedit plugin and no plugin is noarch because gedit is not
> noarch.
That's not a reason python-based plugins must be arched, too. In case of
gedit-plugins arched-pkg is needed because of files have to be (unfortunately)
placed inside %{_libdir}/gedit/plugins/
> Gedit take .plugin in /usr/lib64/gedit/plugins and can take other
> file in /usr/share/gedit plugin.
Would not make things better, anyways and will not work for sure.
> Are you sure for the Germán's recommandation about noarch ?
Not really, so I looked around the web and found this page:
https://live.gnome.org/Gedit/PythonPluginHowTo
So Germán was horribly wrong...
There are some issues about the spec-file I want to discuss with you:
-%global commit a9b65a6358c07e41ea835bcd22cf1c99a8df470a
-%global shortcommit %(c=%{commit}; echo ${c:0:7})
Is not needed, see below.
+# gedit-plugins unfortunately need to be arched because they are installed
+# inside %%{_libdir}. This plugin is written in Python and so cannot
+# provide any debuginfo.
+%global debug_package %{nil}
You wimust switch off generation of debuginfo-pkg in such a case as here
(having just noarched stuff in arched-pkg).
+# gedit requires python(abi) = 3.X, so let's bytecompile with the proper
+# python-interpreter providing this.
+%global __python %{__python3}
You need to byte-compile against the same python-abi which gedit uses.
Byte-compiling against other abi-version is pointless and has no use.
-Summary:Gedit plugin who strip trailing whitespace on save
+Summary:Gedit plugin stripping trailing whitespaces before saving
The summary is written bad english, take my proposal or correct otherwise.
Change the bug, too, please.
-Group: Applications/Editors
Group-Tag was obsoleted around F12 somewhen and actually is need for el5, only.
-URL:http://github.com/jonleighton/gedit-trailsave
+URL:http://github.com/jonleighton/%{name}
Use macros more frequently, please. In this case they will reduce errors caused
through typos.
-Source0:
https://github.com/jonleighton/gedit-trailsave/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz
+Source0:
https://github.com/jonleighton/%{name}/archive/v%{version}.tar.gz#/%{name}-{version}.tar.gz
same goes here for macros. Changing the Source-Url from %commit to versioned
one will make updates easier, actually.
-BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
deprecated, only needed for el5
-BuildRequires: python
+BuildRequires: python3-devel
We actually need python3 here, because of gedit uses python3. Packages Python
is involved in, are mandatory to have BR: pythonX-devel by guidelines.
-Requires: gedit >= 3
Every activly maintained version of Fedora ships gedit v3.X, so requiring an
explict version is obsolete and frowned upon by guidelines.
-Requires: python
+# python3 is pulled from gedit dependencies, so no need to install explicitly
+Requires: gedit%{?_isa}
Proper version of Python is pulled-in by gedit already. Since the plugin
ends-up in an arched-dir, the correctly arched-version of gedit is mandatory,
here.
%description
-Gedit plugin who automatically strip all trailing whitespace before saving.
+%{name} is a plugin for gedit which automatically strips all trailing
+whitespaces before saving the document to disk.
%description is written in horrible english, take my proposal or correct
otherwise, please.
%prep
-%setup -qn %{name}-%{commit}
+%setup -q
There's no need for %commit, anymore. See above...
%build
+# noop
Comment clearly there's no real build-process needed.
%install
-rm -rf %{buildroot}
deleting buildroot is obsoleted and only needed for el5.
-install -d %{buildroot}%{_libdir}/gedit/plugins/
+mkdir -p %{buildroot}%{_libdir}/gedit/plugins/%{name}
Plugins go in %{buildroot}%{_libdir}/gedit/plugins/%{name}
-install trailsave.plugin %{buildroot}%{_libdir}/gedit/plugins/
-install trailsave.py %{buildroot}%{_libdir}/gedit/plugins/
+install -pm 0644 trailsave.plugin %{buildroot}%{_libdir}/gedit/plugins/
+install -pm 0644 trailsave.py %{buildroot}%{_libdir}/gedit/plugins/%{name}/
You should preserve timestamps of file being installed.
-%clean
-rm -rf %{buildroot}
Having a %clean-target is only needed for el5.
%files
-%defattr(-,root,root,-)
Obsoleted, el5 only.
-%{_libdir}/gedit/plugins/trailsave.py
-%{_libdir}/gedit/plugins/trailsave.pyo
-%{_libdir}/gedit/plugins/trailsave.pyc
-%{_libdir}/gedit/plugins/trailsave.plugin
+%{_libdir}/gedit/plugins/*
Using wild-glob is actually shorter and less error prone in case of files
beeing added/removed or renamed and will make sure subdir are owned by rpm
properly.
%ch
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #10 from Guillaume Kulakowski --- I look other Gedit plugin and no plugin is noarch because gedit is not noarch. Gedit take .plugin in /usr/lib64/gedit/plugins and can take other file in /usr/share/gedit plugin. Are you sure for the Germán's recommandation about noarch ? -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=rHIbLWDCZA&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #9 from Germán Racca --- I'm glad you guys are following my stereotypes :) -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=6hCz2qq2BT&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #8 from Björn Esser --- As far as I can see in spec, it's basically unchanged since you reported for review. Please add the changes proposed by Germán in comment #1 and comment #2. I'll start full review then. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=ci2dAXfTxa&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #7 from Björn Esser --- (In reply to Guillaume Kulakowski from comment #6) > > I'll review during this week... > I must apply before the german's recommandation. Yes! Stereotypes must be maintained. ;P > > Spec-Url returns HTML-Document: > Yes, it's a forge. Raw files is here : > https://projects.llaumgui.com/p/rpmbuild/source/file/master/SPECS/gedit- > trailsave.spec fedora-review likes it this way ( More German's stereotypes ahead XD ): Spec URL: https://projects.llaumgui.com/p/rpmbuild/source/file/master/SPECS/gedit-trailsave.spec SRPM URL: http://llaumgui.fedorapeople.org/review/gedit-trailsave/gedit-trailsave-3.1.2-1.fc18.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=BWxsTdkXQb&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #6 from Guillaume Kulakowski --- > I'll review during this week... I must apply before the german's recommandation. > Spec-Url returns HTML-Document: Yes, it's a forge. Raw files is here : https://projects.llaumgui.com/p/rpmbuild/source/file/master/SPECS/gedit-trailsave.spec -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=LEROiL7MDI&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #5 from Björn Esser --- Spec-Url returns HTML-Document: INFO: Downloading .spec and .srpm files error: line 1: Unknown tag: https://bugzilla.redhat.com/token.cgi?t=6KPW48z6fk&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 Björn Esser changed: What|Removed |Added Status|NEW |ASSIGNED CC||bjoern.es...@gmail.com Version|19 |rawhide Assignee|nob...@fedoraproject.org|bjoern.es...@gmail.com Flags||fedora-review? --- Comment #4 from Björn Esser --- I'll review during this week... -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=4bnta2C2KJ&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007 --- Comment #3 from Germán Racca --- > - There is no a separate license file in the source package, so you should > ask > upstream to include it. I think that the license text inside README.md is ok in this case! -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=GgMFdMf796&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007
--- Comment #2 from Germán Racca ---
(In reply to Germán Racca from comment #1)
> - You should create a directory called "trailsave" and install the plugin in
> it: %{_datadir}/trailsave/
Sorry, install dir should be: %{_datadir}/gedit/plugins/trailsave/
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=hhMlUreGdY&a=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 965007] Review Request: gedit-trailsave - Gedit plugin who strip trailing whitespace on save
https://bugzilla.redhat.com/show_bug.cgi?id=965007
Germán Racca changed:
What|Removed |Added
CC||gra...@gmail.com
--- Comment #1 from Germán Racca ---
Hi Gillaume!
Some comments:
- This is a "noarch" package because you don't build anything (rpmlint
complaints E: no-binary), so you must put "BuildArch: noarch" inside the spec
file.
- As this is a noarch plugin, you shouldn't use %{_libdir} as the install dir;
instead you should use %{_datadir}.
- You should create a directory called "trailsave" and install the plugin in
it: %{_datadir}/trailsave/
- You should remove (unless you intend to maintain this package for EPEL5):
- "BuildRoot" tag
- "rm -rf %{buildroot}" from install section
- the "%clean" section
- "%defattr(-,root,root,-)"
- There is no a separate license file in the source package, so you should ask
upstream to include it.
HTH somewhat,
Germán.
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=83GllQwE71&a=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
