| Message ID | cu7h8a19qvq.fsf@systemreboot.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#35684] import: github: Sort releases before picking the latestone. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | Apply failed |
Arun Isaac <arunisaac@systemreboot.net> skribis: > Prior to this, for some packages (e.g., osc in (gnu packages > build-tools)), `guix refresh' could not find the latest version > correctly. Good catch. > From 33f42ea8a33c6e502062708336a5ab8be871c92b Mon Sep 17 00:00:00 2001 > From: Arun Isaac <arunisaac@systemreboot.net> > Date: Sat, 11 May 2019 16:40:38 +0530 > Subject: [PATCH] import: github: Sort releases before picking the latest one. > > * guix/import/github.scm (latest-released-version): Sort releases before > picking the first one as the latest. [...] > + (first > + (sort > + (filter-map > + (lambda (release) > + (let ((tag (or (hash-ref release "tag_name") ;a "release" > + (hash-ref release "name"))) ;a tag > + (name-length (string-length package-name))) > + (cond > + ;; some tags include the name of the package e.g. "fdupes-1.51" > + ;; so remove these > + ((and (< name-length (string-length tag)) > + (string=? (string-append package-name "-") > + (substring tag 0 (+ name-length 1)))) > + (substring tag (+ name-length 1))) > + ;; some tags start with a "v" e.g. "v0.25.0" > + ;; where some are just the version number > + ((string-prefix? "v" tag) > + (substring tag 1)) > + ;; Finally, reject tags that don't start with a digit: > + ;; they may not represent a release. > + ((and (not (string-null? tag)) > + (char-set-contains? char-set:digit > + (string-ref tag 0))) > + tag) > + (else #f)))) > + (match (remove pre-release? json) > + (() json) ; keep everything > + (releases releases))) > + version>?))))) LGTM, but… … while you’re at it, would you mind cleaning this up a bit? :-) Namely, I think this big ‘lambda’ could be given a name and moved out of the way to make ‘latest-released-version’ easier to read. Also, it would probably be reasonable to avoid ‘first’ and instead write: (match (sort …) ((first . _) first) (() (leave (G_ "no releases found etc.~%")))) WDYT? :-) If you’d rather leave that for later, you can also just go ahead and commit your patch. Thanks, Ludo’.
Arun Isaac <arunisaac@systemreboot.net> skribis: > From d3f28de8fedc41732a07edf2ea91222208ccc73f Mon Sep 17 00:00:00 2001 > From: Arun Isaac <arunisaac@systemreboot.net> > Date: Sat, 11 May 2019 16:40:38 +0530 > Subject: [PATCH] import: github: Sort releases before picking the latest one. > > * guix/import/github.scm (latest-released-version): Sort releases before > picking the first one as the latest. Very nice, LGTM! If you don’t mind, you can make it two patches for clarity (one that defines ‘release->version’, and one that adds the call to ‘sort’); otherwise just amend the commit log above to mention ‘release->version’. Thank you! Ludo’.
Arun Isaac <arunisaac@systemreboot.net> skribis: >> If you don’t mind, you can make it two patches for clarity (one that >> defines ‘release->version’, and one that adds the call to ‘sort’) > > Sure, pleae find attached. Thank you, it still LGTM. :-) Ludo’.
Pushed to master, thanks!
From 33f42ea8a33c6e502062708336a5ab8be871c92b Mon Sep 17 00:00:00 2001 From: Arun Isaac <arunisaac@systemreboot.net> Date: Sat, 11 May 2019 16:40:38 +0530 Subject: [PATCH] import: github: Sort releases before picking the latest one. * guix/import/github.scm (latest-released-version): Sort releases before picking the first one as the latest. --- guix/import/github.scm | 55 ++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/guix/import/github.scm b/guix/import/github.scm index 4d12339204..a8af318bc8 100644 --- a/guix/import/github.scm +++ b/guix/import/github.scm @@ -183,32 +183,35 @@ API when using a GitHub token") API. This may be fixed by using an access token and setting the environment variable GUIX_GITHUB_TOKEN, for instance one procured from https://github.com/settings/tokens")) - (any - (lambda (release) - (let ((tag (or (hash-ref release "tag_name") ;a "release" - (hash-ref release "name"))) ;a tag - (name-length (string-length package-name))) - (cond - ;; some tags include the name of the package e.g. "fdupes-1.51" - ;; so remove these - ((and (< name-length (string-length tag)) - (string=? (string-append package-name "-") - (substring tag 0 (+ name-length 1)))) - (substring tag (+ name-length 1))) - ;; some tags start with a "v" e.g. "v0.25.0" - ;; where some are just the version number - ((string-prefix? "v" tag) - (substring tag 1)) - ;; Finally, reject tags that don't start with a digit: - ;; they may not represent a release. - ((and (not (string-null? tag)) - (char-set-contains? char-set:digit - (string-ref tag 0))) - tag) - (else #f)))) - (match (remove pre-release? json) - (() json) ; keep everything - (releases releases)))))) + (first + (sort + (filter-map + (lambda (release) + (let ((tag (or (hash-ref release "tag_name") ;a "release" + (hash-ref release "name"))) ;a tag + (name-length (string-length package-name))) + (cond + ;; some tags include the name of the package e.g. "fdupes-1.51" + ;; so remove these + ((and (< name-length (string-length tag)) + (string=? (string-append package-name "-") + (substring tag 0 (+ name-length 1)))) + (substring tag (+ name-length 1))) + ;; some tags start with a "v" e.g. "v0.25.0" + ;; where some are just the version number + ((string-prefix? "v" tag) + (substring tag 1)) + ;; Finally, reject tags that don't start with a digit: + ;; they may not represent a release. + ((and (not (string-null? tag)) + (char-set-contains? char-set:digit + (string-ref tag 0))) + tag) + (else #f)))) + (match (remove pre-release? json) + (() json) ; keep everything + (releases releases))) + version>?))))) (define (latest-release pkg) "Return an <upstream-source> for the latest release of PKG." -- 2.21.0