Bug 1426844

Until someone more experienced reviews this package, here are my thoughts:

> Summary:		A Linux clone of Notepad++

Refrain from using constructs like this. Notepad++ is proprietary software if I'm correct, and we shouldn't use their name without permission. Why not just qt based advanced text editor. 

> Source0:		https://github.com/notepadqq/notepadqq/archive/v%{version}.tar.gz

Can also be:

> Source0:		%{url}/archive/v%{version}.tar.gz

You can also add #/%{name}-%{version}.tar.gz to the end to have spectool download the file with a pretty file name. 

> Source1:		https://github.com/notepadqq/CodeMirror/archive/5.18.2-nqq.tar.gz

Why is this needed, and what is this? If it's a needed library why not package it separately?

In %description, avoid bullet points.

No need to reference builddir in %setup that's the current directory. You can use tar xf %SOURCE1 to achieve the same. However, you shouldn't be doing this at all. (as stated above)

Make doesn't respect RPM_OPT_FLAGS. Consider using %make_build.

No need to remove RPM buildroot directory. 

Why aren't you using %make_install?

You are patching files with sed after compilation, which seems like a bad practice to me. You should patch before build using %patch. If that's impossible, maybe contact upstream to make the build process more smooth.

You are mentioning node, which isn't listed as a requirement? Does it require a newer version?

------

That's it for me, hopefully someone more experienced can also provide feedback. :)

Cheers,
Nemanja

(In reply to Nemanja Milosevic from comment #1)


Hi and thanks for explaining all
I will answer some of them and fix problems

> Refrain from using constructs like this. Notepad++ is proprietary software if I'm correct, and we shouldn't use their name without permission. Why not just qt based advanced text editor. 

Notepad++ is GPL software, although I will change the name in description for you say

> Source0: ... 
Fixed
About the name, I changed as you write, sounds a bit strange but works

> Source1:		https://github.com/notepadqq/CodeMirror/archive/5.18.2-nqq.tar.gz
>Why is this needed, and what is this? If it's a needed library why not package it separately?

Its a submodule of notepadqq git, github doesnt download it when it downloads the .tar.gz . I didnt think of it as a library because is in it and its like a js file but a lot of it, as an addon. (And without it it doesnt builds)

Corrected description

About the patchs..
The launcher one I sent a better patch to upstream and its merged, but is not released yet (https://github.com/notepadqq/notepadqq/commit/13dec7d5e1008c0d14de56c739d45ec1477f4557 )

The rest, I put it in the prep section, I'm sorry but I dont understand how %patch works

> Make doesn't respect RPM_OPT_FLAGS. Consider using %make_build.
> Why aren't you using %make_install?
I didnt use %make_build and %make_install because I read first time wiki in spanish (I'm Spanish) and it doesnt mention it..
I put %make_build, but it seems that make install doesnt change nothing

> No need to remove RPM buildroot directory.
I think is because same as above, removed line

> You are mentioning node, which isn't listed as a requirement? Does it require a newer version?
It's because for 99% of users notepadqq doesnt need it, and I dont know if its a way to put "Optional" depends

Uploaded changes:
Spec URL: https://mirror.jkanetwork.com/Fedora/SOURCES/notepadqq.spec
SRPM URL: https://mirror.jkanetwork.com/Fedora/SOURCES/notepadqq-1.0.1-5.fc25.src.rpm

Thanks

I reuploaded notepadqq spec and srpm from changes at github from developer at same location

url for more info: https://github.com/notepadqq/notepadqq-packaging/pull/58

Nice to see you working with upstream, I will run the review, it will be nice if you do some informal reviews.

Regards

(In reply to William Moreno from comment #4)
> Nice to see you working with upstream, I will run the review, it will be
> nice if you do some informal reviews.
> 
> Regards

Great :)
I will see it because I don't know much about rpm and reviews, but I will try to test or do things if I can

Hello kevin, do you still want to continúe here?

another friendly ping. I could continue as requester if interest is lost.

I can help sponsor-wise, assuming submitter is available and interested in continuing this review (per comment #6)

Hi, I'm sorry for not answering before, mail of #6 got lost in my inbox
I'm not doing anymore that sorry, anyone that wants to continue is welcome :)

This message is a reminder that Fedora 25 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 25. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '25'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version'
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not
able to fix it before Fedora 25 is end of life. If you would still like
to see this bug fixed and are able to reproduce it against a later version
of Fedora, you are encouraged  change the 'version' to a later Fedora
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's
lifetime, sometimes those efforts are overtaken by events. Often a
more recent Fedora release includes newer upstream software that fixes
bugs or makes them obsolete.

Hello,

I'd like to take over to get this package included with Fedora28. (Sooner would be great, but well.)

Given comment #9, this is a dead review.

If you want to pick things up, go ahead an open a new one.

*** This bug has been marked as a duplicate of bug 1519785 ***