all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Achim Dreyer <ml11045@adreyer.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] SPAM: [PATCH container 1/1] Fix numbering scheme detection for CentOS Stream releases.
Date: Thu, 15 Oct 2020 14:43:18 +0200	[thread overview]
Message-ID: <20201015144318.3beba0f8@rosa.proxmox.com> (raw)
In-Reply-To: <20201013170858.1458272-2-ml11045@adreyer.com>

Thanks for the patch!

I could reproduce the issue by creating a centos container and
installing 'centos-release-stream' and the patch does fix it.

We'd still need a signed CLA from you in order to be able to incorporate
it - see:
https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright

one suggestion inline:

On Tue, 13 Oct 2020 18:08:58 +0100
Achim Dreyer <ml11045@adreyer.com> wrote:

> Signed-off-by: Achim Dreyer <ml11045@adreyer.com>
> ---
>  src/PVE/LXC/Setup/CentOS.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/LXC/Setup/CentOS.pm b/src/PVE/LXC/Setup/CentOS.pm
> index 0825273..aab0488 100644
> --- a/src/PVE/LXC/Setup/CentOS.pm
> +++ b/src/PVE/LXC/Setup/CentOS.pm
> @@ -19,7 +19,7 @@ sub new {
>  
>      my $version;
>  
> -    if ($release =~ m/release\s+(\d+\.\d+)(\.\d+)?/) {
> +    if ($release =~ m/release\s+(\d+)(\.\d+)?(\.\d+)?/) {
I would rather make the 2 matches in 2 separate if-branches
(leave the current one, for regular centos release number (e.g. 'CentOS
Linux release 8.0.1905 (Core)') and add one explicitly matching CentOS streams
(Stream release\s+(\d+))

Mostly because it increases readability (IMHO), but also because currently
the code could match for particular point-releases (7.1), which would get
lost if you only take the first number.


>  	if ($1 >= 5 && $1 <= 9) {
>  	    $version = $1;
>  	}






      reply	other threads:[~2020-10-15 12:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-13 17:08 [pve-devel] SPAM: [PATCH container 0/1] fix numbering scheme detection for CentOS Stream Achim Dreyer
2020-10-13 17:08 ` [pve-devel] SPAM: [PATCH container 1/1] Fix numbering scheme detection for CentOS Stream releases Achim Dreyer
2020-10-15 12:43   ` Stoiko Ivanov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201015144318.3beba0f8@rosa.proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=ml11045@adreyer.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal