[Bug 1426844] Review Request: notepadqq - A Linux clone of Notepad++

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Nemanja Milosevic <nmilosevnm@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nmilosevnm@xxxxxxxxx



--- Comment #1 from Nemanja Milosevic <nmilosevnm@xxxxxxxxx> ---
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

-- 
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@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]