Trim `kerl list releases` #465

paulo-ferraz-oliveira · 2023-09-26T21:17:27Z

Description

... as per the README's Erlang/OTP support policy, and the discussion below.

Closes #459.

I have performed a self-review of my changes
I have read and understood the contributing guidelines

As per our README's Erlang/OTP support policy

kerl

jadeallenx · 2023-09-26T21:42:52Z

I don't think we should artificially limit the list of valid releases but it think it would be helpful to mark which OTP releases we are "officially" supporting somehow - maybe with an asterisk like git branch? Also the list is ridiculously long now - it might be worthwhile to trim some of it that's really quite ancient by default - if you give like "all" as a parameter you get the entire list, otherwise, you get a shorter more recently set of releases by default

paulo-ferraz-oliveira · 2023-09-26T21:56:04Z

Ok, so based on your comment:

we could accept kerl list releases all to have a full list (basically as-is, today, with a potential filter on top - see 2.)
we could change kerl list releases to "a shorter more recently set of releases by default" - do you want a fix "initial version", e.g. latest - 4?
accept kerl list releases with an * for the ones that are in the support policy (and a note, at the bottom stating what the * is)

In any case, do you think otp_releases should always contain all versions, even if we're filtering some for "listing"?

jadeallenx · 2023-09-26T22:20:44Z

Ok, so based on your comment:

we could accept kerl list releases all to have a full list (basically as-is, today, with a potential filter on top - see 2.)

we could change kerl list releases to "a shorter more recently set of releases by default" - do you want a fix "initial version", e.g. latest - 4?

accept kerl list releases with an * for the ones that are in the support policy (and a note, at the bottom stating what the * is)

This all sounds excellent

In any case, do you think otp_releases should always contain all versions, even if we're filtering some for "listing"?

I do think it should be the entire list - I wish there was a way we could not fetch it so often but afaik, there's no HEAD method we could use

garazdawi · 2023-09-27T06:56:35Z

I have some scripts that parse the output of kerl list releases so if you can it would be great to not change the output too much :)

Maybe it could:

remove any release before 17 (those starting with R)
remove any release candidate
list only the latest patch for each release

For example:

> kerl list releases
17.5.6.10
18.3.4.11
19.3.6.13
20.3.8.26
21.3.8.24
22.3.4.26
23.3.4.19
24.3.4.13
25.3.2.6
26.1
Run 'kerl update releases' to update this list from erlang.org
Run 'kerl list releases all' to view all available releases

jadeallenx · 2023-09-27T13:45:32Z

I think @garazdawi is not alone in having some tools that parse kerl's output so I think we need to continue to provide the entire list but I think these suggestions for pruning are helpful and good ones. The pruned list probably should include some text like "the full list of releases is available using the command ..."

paulo-ferraz-oliveira · 2023-09-27T18:36:36Z

I understand the need to have output similar to what there is, right now, but that could potentially be controlled with a variable, which is, I guess, the purpose of all (doesn't that suit you, @garazdawi?). So we'd trim the default but allow keeping behaviour via a control flag. We could then bump the major version and specify why that is, e.g. in the release notes.

paulo-ferraz-oliveira · 2023-09-27T18:45:55Z

Ok, so what I have until now is:

we make (new) kerl list releases all behave the same as (atm) kerl list releases
we (breaking change) update kerl list releases to "a shorter more recently set of releases by default" with a fixed "initial version"
- e.g. latest - 9 (this is an implementation detail we can discuss in the review pane)?
  - what I don't like about this approach is the support policy is OTP-2 but we're printing stuff that's e.g. 17+ (thus the - 9)
- in this list we also remove release candidates, and show only latest patches
- we add, to kerl list releases, an * for the ones that are in the support policy (and a note, at the bottom stating what the * is), but only for the regular list (again, all keeps behaviour, as stated)
otp_releases contain all versions, as-is atm
we add note "Run 'kerl list releases all' to view all available releases" to the bottom of the non-all call

Does this seem to suit all identified use cases, @jadeallenx, @garazdawi?

jadeallenx · 2023-09-27T20:03:31Z

This is a great summary! Yes let’s do this please

paulo-ferraz-oliveira · 2023-09-27T20:05:23Z

@garazdawi, I'm moving in the direction of my previous comment (with @jadeallenx's blessing 😄) . Do let me know if there's something you think could be adjusted, otherwise I can also add you as a reviewer and then we can go over the final result together.

paulo-ferraz-oliveira · 2023-09-27T21:50:02Z

Output for kerl list releases all (culled below):

R13B03
...
R16B03_yielding_binary_to_term
17.0-rc1
...
18.0
...
19.0.1
...
20.1
...
21.2.1
...
22.3.4.26
...
26.1
Run './kerl update releases' to update this list from erlang.org

Output for kerl list releases:

17.5.6.10
18.3.4.11
19.3.6.13
20.3.8.26
21.3.8.24
22.3.4.26
23.3.4.19
* 24.3.4.13
* 25.3.2.6
* 26.1
Run './kerl update releases' to update this list from erlang.org
Run './kerl list releases all' to view all available releases
Note: * means "currently supported"

Given the current implementation, this also means that e.g. when 27.0-rc1 comes along it won't be visible in the non-all list.

garazdawi · 2023-09-28T05:34:12Z

Can we have the star after the version so that it is an easy grep + awk to get the versions?

grep “^[0-9]” | awk '{print $1}'

jadeallenx

This all looks good to me. I think maybe we should only list releases from 20 onwards personally. But I'm willing to hear reasons why we should include older releases (17 has barely baked map support, for example...)

jadeallenx · 2023-09-28T18:03:13Z

kerl

        check_releases
-        list_print "$2"
+        list_print "$2" "$3"
        l=t stderr "Run '$0 update releases' to update this list from erlang.org"


Since we're touching this area of code, I think this message should not mention the erlang.org website any more (since it really parses tags from github by default)

Suggested change

l=t stderr "Run '$0 update releases' to update this list from erlang.org"

l=t stderr "Run '$0 update releases' to update this list"

jadeallenx · 2023-09-28T18:03:54Z

kerl

+                }
+                END {
+                    if (!/-rc/) {
+                        printf "* %s\n",$0


Put the asterisk behind the version tag

Suggested change

printf "* %s\n",$0

printf "%s *\n",$0

I'll accept the suggestion, but also change the README at the same time, so I can't commit from this one 😄

jadeallenx · 2023-09-28T18:06:39Z

I feel like there should be a flag like "KERL_INCLUDE_RELEASE_CANDIDATES=yes" if you want rcs to be listed in your o/p?

paulo-ferraz-oliveira · 2023-09-28T19:45:47Z

I'll add flag KERL_INCLUDE_RELEASE_CANDIDATES.

We keep the behaviour of only showing the highest version in each considered range (whether release candidates are requested or not)

paulo-ferraz-oliveira · 2023-09-28T22:56:59Z

As for the minimum versions (listed or supported) feel free to suggest changes to the variables that define those (instead of 17 it could be 20 or something else). It's a minor detail, to me.

paulo-ferraz-oliveira · 2023-09-28T22:57:50Z

Here's some output (with release candidates)

$ KERL_INCLUDE_RELEASE_CANDIDATES=yes ./kerl list releases
17.0-rc2
17.5.6.10
18.0-rc2
18.3.4.11
19.0-rc2
19.3.6.13
20.0-rc2
20.3.8.26
21.0-rc2
21.3.8.24
22.0-rc3
22.3.4.26
23.0-rc3
23.3.4.19
24.0-rc3 *
24.3.4.13 *
25.0-rc3 *
25.3.2.6 *
26.0-rc3 *
26.1.1 *
Run './kerl update releases' to update this list
Run './kerl list releases all' to view all available releases
Note: * means "currently supported"

paulo-ferraz-oliveira · 2023-09-28T22:58:35Z

Here's some output (without release candidates)

$ ./kerl list releases
17.5.6.10
18.3.4.11
19.3.6.13
20.3.8.26
21.3.8.24
22.3.4.26
23.3.4.19
24.3.4.13 *
25.3.2.6 *
26.1.1 *
Run './kerl update releases' to update this list
Run './kerl list releases all' to view all available releases
Note: * means "currently supported"

paulo-ferraz-oliveira · 2023-09-28T22:59:01Z

Whatever the values that end up in those control variables (for older versions) I feel this is already a major improvement for what we have currently.

jadeallenx · 2023-09-28T23:02:07Z

Whatever the values that end up in those control variables (for older versions) I feel this is already a major improvement for what we have currently.

Yes, no question - this is a lovely improvement. Thanks!

jadeallenx · 2023-09-28T23:15:54Z

We can leave at 17 for now I guess.

paulo-ferraz-oliveira · 2023-09-28T23:18:20Z

Perfect stuff. @garazdawi, feel free to still comment on the final result, even if merged, as I might've not addressed all your concerns. In any case, I'm gonna move forward with other stuff I have prepared...

garazdawi · 2023-09-29T06:51:14Z

lgtm!

paulo-ferraz-oliveira · 2023-09-29T16:28:57Z

And as soon as it was merged it bit me 😄 I started a repo. with @starbelly that pulls kerl from the main branch, and we were using list releases hahaha (now it's using list releases all).

Trim the list of supported releases to <latest>-2

f8d851e

As per our README's Erlang/OTP support policy

paulo-ferraz-oliveira commented Sep 26, 2023

View reviewed changes

kerl Outdated Show resolved Hide resolved

paulo-ferraz-oliveira commented Sep 26, 2023

View reviewed changes

kerl Show resolved Hide resolved

paulo-ferraz-oliveira requested a review from jadeallenx September 26, 2023 21:24

paulo-ferraz-oliveira changed the title ~~Trim the list of supported releases to "latest - 2"~~ Trim kerl list releases Sep 27, 2023

Adapt to more recently discussed requirements

b287b55

Act on CI results: we "broke" the interface

765f58e

jadeallenx reviewed Sep 28, 2023

View reviewed changes

paulo-ferraz-oliveira added 2 commits September 28, 2023 20:45

Act on review comments: tweak output a bit

5ae4131

Introduce KERL_INCLUDE_RELEASE_CANDIDATES to control output

10f1210

We keep the behaviour of only showing the highest version in each considered range (whether release candidates are requested or not)

paulo-ferraz-oliveira requested a review from jadeallenx September 28, 2023 23:02

jadeallenx approved these changes Sep 28, 2023

View reviewed changes

paulo-ferraz-oliveira merged commit 209e0bb into kerl:master Sep 28, 2023

paulo-ferraz-oliveira deleted the unfeature/older-supported-versions branch September 28, 2023 23:18

jadeallenx mentioned this pull request Oct 3, 2023

Fix csh activate/deactivate code, add test suite #470

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trim `kerl list releases` #465

Trim `kerl list releases` #465

paulo-ferraz-oliveira commented Sep 26, 2023 •

edited

Loading

jadeallenx commented Sep 26, 2023

paulo-ferraz-oliveira commented Sep 26, 2023

jadeallenx commented Sep 26, 2023

garazdawi commented Sep 27, 2023

jadeallenx commented Sep 27, 2023 •

edited

Loading

paulo-ferraz-oliveira commented Sep 27, 2023

paulo-ferraz-oliveira commented Sep 27, 2023

jadeallenx commented Sep 27, 2023

paulo-ferraz-oliveira commented Sep 27, 2023

paulo-ferraz-oliveira commented Sep 27, 2023

garazdawi commented Sep 28, 2023

jadeallenx left a comment

jadeallenx Sep 28, 2023

paulo-ferraz-oliveira Sep 28, 2023

jadeallenx Sep 28, 2023

paulo-ferraz-oliveira Sep 28, 2023

paulo-ferraz-oliveira Sep 28, 2023

jadeallenx commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

jadeallenx commented Sep 28, 2023

jadeallenx commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

garazdawi commented Sep 29, 2023

paulo-ferraz-oliveira commented Sep 29, 2023

	l=t stderr "Run '$0 update releases' to update this list from erlang.org"
	l=t stderr "Run '$0 update releases' to update this list"

Trim kerl list releases #465

Trim kerl list releases #465

Conversation

paulo-ferraz-oliveira commented Sep 26, 2023 • edited Loading

Description

jadeallenx commented Sep 26, 2023

paulo-ferraz-oliveira commented Sep 26, 2023

jadeallenx commented Sep 26, 2023

garazdawi commented Sep 27, 2023

jadeallenx commented Sep 27, 2023 • edited Loading

paulo-ferraz-oliveira commented Sep 27, 2023

paulo-ferraz-oliveira commented Sep 27, 2023

jadeallenx commented Sep 27, 2023

paulo-ferraz-oliveira commented Sep 27, 2023

paulo-ferraz-oliveira commented Sep 27, 2023

garazdawi commented Sep 28, 2023

jadeallenx left a comment

Choose a reason for hiding this comment

jadeallenx Sep 28, 2023

Choose a reason for hiding this comment

paulo-ferraz-oliveira Sep 28, 2023

Choose a reason for hiding this comment

jadeallenx Sep 28, 2023

Choose a reason for hiding this comment

paulo-ferraz-oliveira Sep 28, 2023

Choose a reason for hiding this comment

paulo-ferraz-oliveira Sep 28, 2023

Choose a reason for hiding this comment

jadeallenx commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

jadeallenx commented Sep 28, 2023

jadeallenx commented Sep 28, 2023

paulo-ferraz-oliveira commented Sep 28, 2023

garazdawi commented Sep 29, 2023

paulo-ferraz-oliveira commented Sep 29, 2023

Trim `kerl list releases` #465

Trim `kerl list releases` #465

paulo-ferraz-oliveira commented Sep 26, 2023 •

edited

Loading

jadeallenx commented Sep 27, 2023 •

edited

Loading