* [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
@ 2021-07-21 16:47 Patrice Clement
2021-07-21 17:16 ` Ulrich Mueller
2021-07-22 3:44 ` Sam James
0 siblings, 2 replies; 8+ messages in thread
From: Patrice Clement @ 2021-07-21 16:47 UTC (permalink / raw
To: gentoo-dev; +Cc: vim
... and replace it with a test against the REPLACING_VERSIONS variable.
See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
Patch courtesy of Arfrever.
Closes: https://github.com/gentoo/gentoo/pull/21733
Signed-off-by: Patrice Clement <monsieurp@gentoo.org>
---
eclass/vim-plugin.eclass | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index 6b72d66111d..460edcce3eb 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -126,31 +126,31 @@ update_vim_afterscripts() {
display_vim_plugin_help() {
local h
- if ! has_version ${CATEGORY}/${PN} ; then
- if [[ -n "${VIM_PLUGIN_HELPFILES}" ]] ; then
+ if [[ -z ${REPLACING_VERSIONS} ]]; then
+ if [[ -n "${VIM_PLUGIN_HELPFILES}" ]]; then
elog " "
elog "This plugin provides documentation via vim's help system. To"
elog "view it, use:"
- for h in ${VIM_PLUGIN_HELPFILES} ; do
+ for h in ${VIM_PLUGIN_HELPFILES}; do
elog " :help ${h}"
done
elog " "
- elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]] ; then
+ elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]]; then
elog " "
while read h ; do
elog "$h"
done <<<"${VIM_PLUGIN_HELPTEXT}"
elog " "
- elif [[ -n "${VIM_PLUGIN_HELPURI}" ]] ; then
+ elif [[ -n "${VIM_PLUGIN_HELPURI}" ]]; then
elog " "
elog "Documentation for this plugin is available online at:"
elog " ${VIM_PLUGIN_HELPURI}"
elog " "
fi
- if has "filetype" "${VIM_PLUGIN_MESSAGES}" ; then
+ if has "filetype" "${VIM_PLUGIN_MESSAGES}"; then
elog "This plugin makes use of filetype settings. To enable these,"
elog "add lines like:"
elog " filetype plugin on"
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
2021-07-21 16:47 [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition Patrice Clement
@ 2021-07-21 17:16 ` Ulrich Mueller
2021-07-22 8:27 ` Patrice Clement
2021-07-22 3:44 ` Sam James
1 sibling, 1 reply; 8+ messages in thread
From: Ulrich Mueller @ 2021-07-21 17:16 UTC (permalink / raw
To: Patrice Clement; +Cc: gentoo-dev, vim
[-- Attachment #1: Type: text/plain, Size: 189 bytes --]
>>>>> On Wed, 21 Jul 2021, Patrice Clement wrote:
> + if [[ -n "${VIM_PLUGIN_HELPFILES}" ]]; then
Quotation marks are not necessary here (and three more times below,
in the elif lines).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
2021-07-21 16:47 [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition Patrice Clement
2021-07-21 17:16 ` Ulrich Mueller
@ 2021-07-22 3:44 ` Sam James
2021-07-22 8:32 ` Patrice Clement
1 sibling, 1 reply; 8+ messages in thread
From: Sam James @ 2021-07-22 3:44 UTC (permalink / raw
To: gentoo-dev; +Cc: vim
[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]
> On 21 Jul 2021, at 17:47, Patrice Clement <monsieurp@gentoo.org> wrote:
>
> ... and replace it with a test against the REPLACING_VERSIONS variable.
>
> See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
>
> Patch courtesy of Arfrever.
>
Ideally, we'd use Arfrever's sign off, but this is kind of a grey
area, so it's not a blocker at all right now.
> Closes: https://github.com/gentoo/gentoo/pull/21733
> Signed-off-by: Patrice Clement <monsieurp@gentoo.org>
> ---
> eclass/vim-plugin.eclass | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> index 6b72d66111d..460edcce3eb 100644
> --- a/eclass/vim-plugin.eclass
> +++ b/eclass/vim-plugin.eclass
> [snip]
The patch itself seems fine, but I have some suggestions while we're
working on the eclass:
- Add EAPI 8 support
- Please drop obsolete EAPI compatibility hacks like:
>has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}"
This is unnecessary because we're only declaring support for EAPI 6 and 7 right now.
- Could you please convert POSIX tests to Bash tests? [0]
- In update_vim_afterscripts, the einfo seems (at first glance) to be misleading. Should
it be printing ${afterdir} instead?
- Ideally, we'd add the standard inherit guard if possible (see e.g. gnuconfig.eclass for a simple example).
[0] https://devmanual.gentoo.org/tools-reference/bash/#single-versus-double-brackets-in-bash
Let me know if you need any assistance, etc.
thanks,
sam
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
2021-07-21 17:16 ` Ulrich Mueller
@ 2021-07-22 8:27 ` Patrice Clement
0 siblings, 0 replies; 8+ messages in thread
From: Patrice Clement @ 2021-07-22 8:27 UTC (permalink / raw
To: gentoo-dev; +Cc: vim
Wednesday 21 Jul 2021 19:16:36, Ulrich Mueller wrote :
> >>>>> On Wed, 21 Jul 2021, Patrice Clement wrote:
>
> > + if [[ -n "${VIM_PLUGIN_HELPFILES}" ]]; then
>
> Quotation marks are not necessary here (and three more times below,
> in the elif lines).
Thanks, this has been mended.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
2021-07-22 3:44 ` Sam James
@ 2021-07-22 8:32 ` Patrice Clement
2021-07-22 21:24 ` Sam James
0 siblings, 1 reply; 8+ messages in thread
From: Patrice Clement @ 2021-07-22 8:32 UTC (permalink / raw
To: gentoo-dev; +Cc: vim
Thursday 22 Jul 2021 04:44:39, Sam James wrote :
>
>
> > On 21 Jul 2021, at 17:47, Patrice Clement <monsieurp@gentoo.org> wrote:
> >
> > ... and replace it with a test against the REPLACING_VERSIONS variable.
> >
> > See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
> >
> > Patch courtesy of Arfrever.
> >
>
> Ideally, we'd use Arfrever's sign off, but this is kind of a grey
> area, so it's not a blocker at all right now.
>
> > Closes: https://github.com/gentoo/gentoo/pull/21733
> > Signed-off-by: Patrice Clement <monsieurp@gentoo.org>
> > ---
> > eclass/vim-plugin.eclass | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> > index 6b72d66111d..460edcce3eb 100644
> > --- a/eclass/vim-plugin.eclass
> > +++ b/eclass/vim-plugin.eclass
> > [snip]
>
> The patch itself seems fine, but I have some suggestions while we're
> working on the eclass:
>
> - Add EAPI 8 support
>
> - Please drop obsolete EAPI compatibility hacks like:
> >has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}"
>
> This is unnecessary because we're only declaring support for EAPI 6 and 7 right now.
>
> - Could you please convert POSIX tests to Bash tests? [0]
>
> - In update_vim_afterscripts, the einfo seems (at first glance) to be misleading. Should
> it be printing ${afterdir} instead?
>
> - Ideally, we'd add the standard inherit guard if possible (see e.g. gnuconfig.eclass for a simple example).
>
> [0] https://devmanual.gentoo.org/tools-reference/bash/#single-versus-double-brackets-in-bash
>
> Let me know if you need any assistance, etc.
>
> thanks,
> sam
>
>
>
>
Thanks for the suggestions. If you don't mind, I'd like to stick to the
original changes for now and address your suggestions later in a follow up
patch.
Best,
^ permalink raw reply [flat|nested] 8+ messages in thread
* [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
@ 2021-07-22 8:54 Patrice Clement
2021-07-23 5:35 ` Patrice Clement
0 siblings, 1 reply; 8+ messages in thread
From: Patrice Clement @ 2021-07-22 8:54 UTC (permalink / raw
To: gentoo-dev; +Cc: vim
... and replace it with a test against the REPLACING_VERSIONS variable.
See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
This is an updated version of the same patch discussed previously on this ML.
Suggested by Arfrever.
---
eclass/vim-plugin.eclass | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index 6b72d66111d..460edcce3eb 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -126,31 +126,31 @@ update_vim_afterscripts() {
display_vim_plugin_help() {
local h
- if ! has_version ${CATEGORY}/${PN} ; then
- if [[ -n "${VIM_PLUGIN_HELPFILES}" ]] ; then
+ if [[ -z ${REPLACING_VERSIONS} ]]; then
+ if [[ -n ${VIM_PLUGIN_HELPFILES} ]]; then
elog " "
elog "This plugin provides documentation via vim's help system. To"
elog "view it, use:"
- for h in ${VIM_PLUGIN_HELPFILES} ; do
+ for h in ${VIM_PLUGIN_HELPFILES}; do
elog " :help ${h}"
done
elog " "
- elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]] ; then
+ elif [[ -n ${VIM_PLUGIN_HELPTEXT} ]]; then
elog " "
while read h ; do
elog "$h"
done <<<"${VIM_PLUGIN_HELPTEXT}"
elog " "
- elif [[ -n "${VIM_PLUGIN_HELPURI}" ]] ; then
+ elif [[ -n ${VIM_PLUGIN_HELPURI} ]]; then
elog " "
elog "Documentation for this plugin is available online at:"
elog " ${VIM_PLUGIN_HELPURI}"
elog " "
fi
- if has "filetype" "${VIM_PLUGIN_MESSAGES}" ; then
+ if has filetype ${VIM_PLUGIN_MESSAGES}; then
elog "This plugin makes use of filetype settings. To enable these,"
elog "add lines like:"
elog " filetype plugin on"
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
2021-07-22 8:32 ` Patrice Clement
@ 2021-07-22 21:24 ` Sam James
0 siblings, 0 replies; 8+ messages in thread
From: Sam James @ 2021-07-22 21:24 UTC (permalink / raw
To: gentoo-dev; +Cc: vim
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
> On 22 Jul 2021, at 09:32, Patrice Clement <monsieurp@gentoo.org> wrote:
>
> Thursday 22 Jul 2021 04:44:39, Sam James wrote :
>>
>>> [snip]
>>
>> The patch itself seems fine, but I have some suggestions while we're
>> working on the eclass:
>>
>> [snip]
>> Let me know if you need any assistance, etc.
>>
>> thanks,
>> sam
>>
> Thanks for the suggestions. If you don't mind, I'd like to stick to the
> original changes for now and address your suggestions later in a follow up
> patch.
It's preferred [0] where, if we know more changes are coming, to batch them up,
unless this is really critical. This is because of needless cache regeneration.
It's generally not a reason to block fixing something if no other issues exist, but it
is a reason to stop and pause/reflect if there are any other low-hanging fruit we could fix while there.
A lot of this is straightforward tidying. I don't mind if you're sure you will be able to return
to it shortly, but then I figure, why not just do it now?
[0] https://devmanual.gentoo.org/eclass-writing/index.html#adding-and-updating-eclasses (see the "Note")
thanks,
sam
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition.
2021-07-22 8:54 Patrice Clement
@ 2021-07-23 5:35 ` Patrice Clement
0 siblings, 0 replies; 8+ messages in thread
From: Patrice Clement @ 2021-07-23 5:35 UTC (permalink / raw
To: gentoo-dev; +Cc: vim
Thursday 22 Jul 2021 10:54:02, Patrice Clement wrote :
> ... and replace it with a test against the REPLACING_VERSIONS variable.
>
> See https://projects.gentoo.org/pms/8/pms.html#x1-10900011.1.
>
> This is an updated version of the same patch discussed previously on this ML.
>
> Suggested by Arfrever.
> ---
> eclass/vim-plugin.eclass | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> index 6b72d66111d..460edcce3eb 100644
> --- a/eclass/vim-plugin.eclass
> +++ b/eclass/vim-plugin.eclass
> @@ -126,31 +126,31 @@ update_vim_afterscripts() {
> display_vim_plugin_help() {
> local h
>
> - if ! has_version ${CATEGORY}/${PN} ; then
> - if [[ -n "${VIM_PLUGIN_HELPFILES}" ]] ; then
> + if [[ -z ${REPLACING_VERSIONS} ]]; then
> + if [[ -n ${VIM_PLUGIN_HELPFILES} ]]; then
> elog " "
> elog "This plugin provides documentation via vim's help system. To"
> elog "view it, use:"
> - for h in ${VIM_PLUGIN_HELPFILES} ; do
> + for h in ${VIM_PLUGIN_HELPFILES}; do
> elog " :help ${h}"
> done
> elog " "
>
> - elif [[ -n "${VIM_PLUGIN_HELPTEXT}" ]] ; then
> + elif [[ -n ${VIM_PLUGIN_HELPTEXT} ]]; then
> elog " "
> while read h ; do
> elog "$h"
> done <<<"${VIM_PLUGIN_HELPTEXT}"
> elog " "
>
> - elif [[ -n "${VIM_PLUGIN_HELPURI}" ]] ; then
> + elif [[ -n ${VIM_PLUGIN_HELPURI} ]]; then
> elog " "
> elog "Documentation for this plugin is available online at:"
> elog " ${VIM_PLUGIN_HELPURI}"
> elog " "
> fi
>
> - if has "filetype" "${VIM_PLUGIN_MESSAGES}" ; then
> + if has filetype ${VIM_PLUGIN_MESSAGES}; then
> elog "This plugin makes use of filetype settings. To enable these,"
> elog "add lines like:"
> elog " filetype plugin on"
> --
> 2.31.1
>
>
Merged. Thanks ulm for the review!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-23 5:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-21 16:47 [gentoo-dev] [PATCH] eclass/vim-plugin.eclass: delete if has_version condition Patrice Clement
2021-07-21 17:16 ` Ulrich Mueller
2021-07-22 8:27 ` Patrice Clement
2021-07-22 3:44 ` Sam James
2021-07-22 8:32 ` Patrice Clement
2021-07-22 21:24 ` Sam James
-- strict thread matches above, loose matches on Subject: below --
2021-07-22 8:54 Patrice Clement
2021-07-23 5:35 ` Patrice Clement
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox