[aur-general] [RFC] freeplane-git PKGBUILD
Hello, Just made a git version of freeplane package I maintain. Since it's one of my first git packages I'd appreciate a second eye / opinion. Here is the link: https://github.com/inglor/pkgbuild/tree/master/freeplane-git Thanks Leonidas
On Thursday 07 November 2013 at 11:38:52, Leonidas wrote:
Hello,
Just made a git version of freeplane package I maintain. Since it's one of my first git packages I'd appreciate a second eye / opinion. Here is the link: https://github.com/inglor/pkgbuild/tree/master/freeplane-git
Thanks Leonidas
Hi! Looks pretty good; I've got a few comments here and there.
source=('git+https://github.com/freeplane/freeplane.git' 'license.txt' 'freeplane.desktop' 'freeplane.run')
It's better to use 'git://' scheme (slightly more intelligent protocol is used). (Note: 'git+' will be unneeded if you switch to 'git://'.)
for file in $( find plugins -type f ) ; do
This construct is whitespace-error-prone. It's a bit better to use find plugins -type f | while read file; do instead of that line.
# Where's the licence? #install -Dm644 license.txt ${pkgdir}/usr/share/freeplane/licence.txt
I suppose you're asking for help with destination (the source is, obviously, under $srcdir). In Arch, custom licenses shall be installed under /usr/share/licenses/$pkgname/, but it is not needed in this package since you have specified license=('GPL'). Regards, -- Ivan Shapovalov / intelfx /
On Thursday 07 November 2013 at 19:21:19, Ivan wrote:
On Thursday 07 November 2013 at 11:38:52, Leonidas wrote:
Hello,
Just made a git version of freeplane package I maintain. Since it's one of my first git packages I'd appreciate a second eye / opinion. Here is the link: https://github.com/inglor/pkgbuild/tree/master/freeplane-git
Thanks Leonidas
Hi!
Looks pretty good; I've got a few comments here and there.
source=('git+https://github.com/freeplane/freeplane.git' 'license.txt' 'freeplane.desktop' 'freeplane.run')
It's better to use 'git://' scheme (slightly more intelligent protocol is used). (Note: 'git+' will be unneeded if you switch to 'git://'.)
for file in $( find plugins -type f ) ; do
This construct is whitespace-error-prone. It's a bit better to use
find plugins -type f | while read file; do
instead of that line.
# Where's the licence? #install -Dm644 license.txt ${pkgdir}/usr/share/freeplane/licence.txt
I suppose you're asking for help with destination (the source is, obviously, under $srcdir).
In Arch, custom licenses shall be installed under /usr/share/licenses/$pkgname/, but it is not needed in this package since you have specified license=('GPL').
And the last one (sorry for spamming): it's better to double-quote all pathes containing "$srcdir" and "$pkgdir" (as well as any variables, e. g. $file) to prevent whitespace errors. Regards, -- Ivan Shapovalov / intelfx /
Go Ivan, Thanks for the comments! On 7 Nov 2013 15:21, "Ivan Shapovalov" <intelfx100@gmail.com> wrote:
Hi!
Looks pretty good; I've got a few comments here and there.
source=('git+https://github.com/freeplane/freeplane.git' 'license.txt'
'freeplane.desktop' 'freeplane.run')
It's better to use 'git://' scheme (slightly more intelligent protocol is
used).
(Note: 'git+' will be unneeded if you switch to 'git://'.)
OK I will try that
for file in $( find plugins -type f ) ; do
This construct is whitespace-error-prone. It's a bit better to use
find plugins -type f | while read file; do
instead of that line.
# Where's the licence? #install -Dm644 license.txt ${pkgdir}/usr/share/freeplane/licence.txt
I suppose you're asking for help with destination (the source is, obviously, under $srcdir).
In Arch, custom licenses shall be installed under /usr/share/licenses/$pkgname/, but it is not needed in this package since you have specified
Thanks I didn't think of that. I'll use your proposal for quoting them as well. license=('GPL').
Well kinda.. If you see the distribution packages from sourceforge there is a licence included. When you build from source I see no licence somewhere in the source code. So the comment is for me to remember there is no licence in source code. I know the package is GPL from the site so I put it there in the licence field. I think it can go away..
Regards,
-- Ivan Shapovalov / intelfx /
Regards, Leonidas
I would discourage the use of all-caps variable names. Shells typically use all-caps for their environment variables, and although I don't know of any shells that use _BUILD_DIR or _APPNAME as environment vars, the possibility exists. On Thu, Nov 7, 2013 at 10:38 AM, Leonidas Spyropoulos <artafinde@gmail.com>wrote:
Go Ivan,
Thanks for the comments!
On 7 Nov 2013 15:21, "Ivan Shapovalov" <intelfx100@gmail.com> wrote:
Hi!
Looks pretty good; I've got a few comments here and there.
source=('git+https://github.com/freeplane/freeplane.git' 'license.txt'
'freeplane.desktop' 'freeplane.run')
It's better to use 'git://' scheme (slightly more intelligent protocol is
used).
(Note: 'git+' will be unneeded if you switch to 'git://'.)
OK I will try that
for file in $( find plugins -type f ) ; do
This construct is whitespace-error-prone. It's a bit better to use
find plugins -type f | while read file; do
instead of that line.
Thanks I didn't think of that. I'll use your proposal for quoting them as well.
# Where's the licence? #install -Dm644 license.txt ${pkgdir}/usr/share/freeplane/licence.txt
I suppose you're asking for help with destination (the source is, obviously, under $srcdir).
In Arch, custom licenses shall be installed under /usr/share/licenses/$pkgname/, but it is not needed in this package since you have specified license=('GPL').
Well kinda.. If you see the distribution packages from sourceforge there is a licence included. When you build from source I see no licence somewhere in the source code. So the comment is for me to remember there is no licence in source code. I know the package is GPL from the site so I put it there in the licence field. I think it can go away..
Regards,
-- Ivan Shapovalov / intelfx /
Regards, Leonidas
On Thu, Nov 7, 2013 at 5:07 PM, Jeremy Audet <ichimonji10@gmail.com> wrote:
I would discourage the use of all-caps variable names. Shells typically use all-caps for their environment variables, and although I don't know of any shells that use _BUILD_DIR or _APPNAME as environment vars, the possibility exists.
On Thu, Nov 7, 2013 at 10:38 AM, Leonidas Spyropoulos <artafinde@gmail.com>wrote:
Go Ivan,
Thanks for the comments!
On 7 Nov 2013 15:21, "Ivan Shapovalov" <intelfx100@gmail.com> wrote:
Hi!
Looks pretty good; I've got a few comments here and there.
source=('git+https://github.com/freeplane/freeplane.git'
'license.txt' 'freeplane.desktop' 'freeplane.run')
It's better to use 'git://' scheme (slightly more intelligent
protocol is used).
(Note: 'git+' will be unneeded if you switch to 'git://'.)
OK I will try that
for file in $( find plugins -type f ) ; do
This construct is whitespace-error-prone. It's a bit better to use
find plugins -type f | while read file; do
instead of that line.
Thanks I didn't think of that. I'll use your proposal for quoting them as well.
# Where's the licence? #install -Dm644 license.txt ${pkgdir}/usr/share/freeplane/licence.txt
I suppose you're asking for help with destination (the source is, obviously, under $srcdir).
In Arch, custom licenses shall be installed under /usr/share/licenses/$pkgname/, but it is not needed in this package since you have specified license=('GPL').
Well kinda.. If you see the distribution packages from sourceforge there is a licence included. When you build from source I see no licence somewhere in the source code. So the comment is for me to remember there is no licence in source code. I know the package is GPL from the site so I put it there in the licence field. I think it can go away..
Regards,
-- Ivan Shapovalov / intelfx /
Regards, Leonidas
Furthermore, _APPNAME is easily replaced with ${pkgname%-*}, and there's hardly any need to create a _BUILD_DIR variable if you're only going to use it once. BTW, please reply at the bottom of the mail Jeremy, no top posting on the mailing lists ;) Cheers, -- Maxime
On 7 Nov 2013 16:31, "Maxime Gauduin" <alucryd@gmail.com> wrote:
Furthermore, _APPNAME is easily replaced with ${pkgname%-*}, and there's
hardly any need to create a _BUILD_DIR variable if you're only going to use it once.
BTW, please reply at the bottom of the mail Jeremy, no top posting on the
mailing lists ;)
Cheers, -- Maxime
Thanks all, I amended as proposed. Cheers, Leonidas
participants (4)
-
Ivan Shapovalov
-
Jeremy Audet
-
Leonidas Spyropoulos
-
Maxime Gauduin