Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
Luciana Lima Brito writes: > On Thu, 15 Apr 2021 09:46:12 +0100 > Christopher Baines wrote: > > Hi, > >> I'm not quite sure how to apply this, I'd suggest using git >> format-patch to generate the file next time as I think there would >> normally be some metadata along with the diff. > > I tried using git format-patch and I got 7 patches from my 7 commits, > then I generate a single patch output, which is attached. > The last commit before my modifications is this: > 410f58cb43f083623885a430700c6818a187cadc Ok, I looked at the overall diff, and it looks to me like these changes should probably be one commit. >> I would consider whether it's useful to have all these let blocks, and >> whether here is the right place for them. > >> Taking a binding like outputs, it's only used in a later let. You can >> do something like this (with let*) to remove the need to have >> multiple let blocks. > >> Also, since matched-outputs is only used when rendering the JSON, I'd >> move all the bindings that are only used for the JSON output within >> that part of the case statement, so that it's clearer that they only >> apply to that bit of the code. >> >> Does that make sense? > > I did it, I used the let* and this helped a lot. I also moved > everything into the case branch of the json. > >> I'm not sure what revision here referrs to. > > It was a placeholder, but now I removed it. > >> I hope that helps, just let me know if you have any questions, > > The function get-derivation-data does not depend on anything, don't you > think it goes better in another place outside render-compare/derivation? On the get-derivation-data function, I wouldn't use the same function to process the inputs, outputs and sources. The data for each is different, so I would separate the code as well. To avoid having to call a procedure three times, on the base, target and common items, I'd consider following the same pattern in the HTML generating code, map over a list of the lists, so something like: (map (lambda (name data) (cons name (match data ((name path hash-alg hash recursive) ... '(base target common) (list (assq-ref outputs 'base) (assq-ref outputs 'target) (assq-ref outputs 'common))) Does that make sense? signature.asc Description: PGP signature
Re: Please review blog post draft: powerpc64le-linux support
On Mon, 2021-04-12 at 12:46 -0700, Chris Marusich wrote: > Hi, > > Chris Marusich writes: > > > This is the final draft, I think. I intend to commit it to the > > "posts" > > directory in guix-artwork on Monday morning, USA time, at which > > point I > > believe it will automatically show up on the blog. > > I have published it in commit > 0129dd529347bfefee96644ac9fbabc29adbe772. > Thank you again to everyone for your help! > Awesome! I also sent an email to Micheal from Phoronix earlier, but it seems they either didnt take it into account or didnt see it for now. Léo signature.asc Description: This is a digitally signed message part
Re: [Outreachy] - Guix Data Service - Set a more informative page title
Canan Talayhan writes: > Hi team, > > Thanks for your help. > > @Chris > After all the modifications that I've made according to your comments, > I've created the latest version of my patch. > Could you please review the patch attached and share your ideas? > > Please note that a few parts are left. After your confirmation, I can > handle it shortly. Thanks Canan, I've had a quick look: > +(define page-header "Dumps") > + > (define (view-dumps available-dumps) >(layout > + #:title > + page-header > #:body > `(,(header) > (div > @@ -31,7 +35,7 @@ > (@ (class "row")) > (div > (@ (class "col-sm-12")) > -(h1 "Database dumps"))) > +(h1 ,page-header))) I'd generally stick with more descriptive titles, "Database dumps" is far clearer than "Dumps" in this case. > + (define page-header "Recent Events") > + >(layout > + #:title > + page-header > #:body > `(,(header) > (div > @@ -200,7 +208,7 @@ > (@ (class "col-sm-12")) > (a (@ (href "/jobs")) > (h3 "Jobs")) > -(h1 "Recent events"))) > +(h1 ,page-header))) The capitalisation used here "events" rather than "Events" is intentional, I wouldn't change it while looking at the titles at least. > (define (view-job job-id query-parameters log) > + (define page-header "Job") > + >(layout > + #:title > + (string-append page-header " " job-id) > #:body > `(,(header) > (div > @@ -339,7 +356,7 @@ > (@ (class "row")) > (div > (@ (class "col-sm-12")) > -(h1 "Job " ,job-id))) > +(h1 ,page-header ," " ,job-id))) I'd be tempted to combine the Job string and the job-id then use it for both the #:title and h1 element. > + (string-append page-header " " (string-take commit-hash 7) " Channel News > Entries") This title looks good, I'd put the more specific bits nearer the start though, so I'd put "Channel News Entries" first, then the Revision bit second. Also, watch out for the line length, I'd indent this so it's not longer than 80 lines. signature.asc Description: PGP signature
Re: OUTREACHY INTERNSHIP - INTRODUCING MYSELF
On Wed, 14 Apr 2021 07:12:51 +0100 obaseki osakpolor wrote: > Hello everyone, > > *I am Obaseki, Osakpolor from Lagos, Nigeria. An Outreachy internship > candidate*. > I am interested in working with you and contributing to the project > - *Guix Data Service: revision processing instrumentation and > performance.* > > While I have lots of experience using git, writing code with Java, > Kotlin, and SQL databases > I am new to contributing to FOSS, Guile, and Guix. > > I look forward to the experience. > Guile seems pretty interesting to work with so far, and guix has been > a delight to use. > I am looking forward to hearing from and working with everyone. > > Regards, > Osakpolor. Hi, and welcome to Guix! UwU
Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
On Thu, 15 Apr 2021 09:46:12 +0100 Christopher Baines wrote: Hi, > I'm not quite sure how to apply this, I'd suggest using git > format-patch to generate the file next time as I think there would > normally be some metadata along with the diff. I tried using git format-patch and I got 7 patches from my 7 commits, then I generate a single patch output, which is attached. The last commit before my modifications is this: 410f58cb43f083623885a430700c6818a187cadc > I would consider whether it's useful to have all these let blocks, and > whether here is the right place for them. > Taking a binding like outputs, it's only used in a later let. You can > do something like this (with let*) to remove the need to have > multiple let blocks. > Also, since matched-outputs is only used when rendering the JSON, I'd > move all the bindings that are only used for the JSON output within > that part of the case statement, so that it's clearer that they only > apply to that bit of the code. > > Does that make sense? I did it, I used the let* and this helped a lot. I also moved everything into the case branch of the json. > I'm not sure what revision here referrs to. It was a placeholder, but now I removed it. > I hope that helps, just let me know if you have any questions, The function get-derivation-data does not depend on anything, don't you think it goes better in another place outside render-compare/derivation? -- Best Regards, Luciana Lima Brito MSc. in Computer Science >From c7af970c677a8d97cb4841a748e343c03e0bc886 Mon Sep 17 00:00:00 2001 From: Luciana Brito Date: Sun, 11 Apr 2021 11:06:06 -0300 Subject: [PATCH 1/7] Include base-derivation and target-derivation on json of render-compare/derivation --- guix-data-service/web/compare/controller.scm | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm index a6aa198..355ce03 100644 --- a/guix-data-service/web/compare/controller.scm +++ b/guix-data-service/web/compare/controller.scm @@ -589,7 +589,12 @@ mime-types) ((application/json) (render-json - '((error . "unimplemented")) ; TODO + `((revision + . ((base + . ((derivation . ,base-derivation))) +(target + . ((derivation . ,target-derivation)) + ;'((error . "unimplemented")) ; TODO #:extra-headers http-headers-for-unchanging-content)) (else (render-html -- 2.30.2 >From d91856c6b4a0e998288b2291f9a9f420f9916dc4 Mon Sep 17 00:00:00 2001 From: Luciana Brito Date: Mon, 12 Apr 2021 11:05:20 -0300 Subject: [PATCH 2/7] Include base and target outputs to json of render-compare/derivation --- guix-data-service/web/compare/controller.scm | 127 --- 1 file changed, 109 insertions(+), 18 deletions(-) diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm index 355ce03..2f54310 100644 --- a/guix-data-service/web/compare/controller.scm +++ b/guix-data-service/web/compare/controller.scm @@ -584,24 +584,115 @@ (derivation-differences-data conn base-derivation target-derivation) - (case (most-appropriate-mime-type - '(application/json text/html) - mime-types) -((application/json) - (render-json - `((revision - . ((base - . ((derivation . ,base-derivation))) -(target - . ((derivation . ,target-derivation)) - ;'((error . "unimplemented")) ; TODO - #:extra-headers http-headers-for-unchanging-content)) -(else - (render-html - #:sxml (compare/derivation - query-parameters - data) - #:extra-headers http-headers-for-unchanging-content))) + + (let ((outputs (assq-ref data 'outputs)) +(inputs (assq-ref data 'inputs)) +(sources (assq-ref data 'sources)) +(system (assq-ref data 'system)) +(build-and-args (assq-ref data 'build-and-args)) +(envronment-and-variables (assq-ref data 'envronment-and-variables)) +) +(let ((base-outputs (assq-ref outputs 'base)) + (target-outputs (assq-ref outputs 'target)) + + (base-inputs (assq-ref inputs 'base)) + (target-inputs (assq-ref inputs 'target)) + + (base-sources (assq-ref sources 'base)) + (target-sources (assq-ref sources 'target)) + + (base-system (assq-ref system
Re: [Outreachy] - Guix Data Service - Set a more informative page title
Hi team,
Thanks for your help.
@Chris
After all the modifications that I've made according to your comments,
I've created the latest version of my patch.
Could you please review the patch attached and share your ideas?
Please note that a few parts are left. After your confirmation, I can
handle it shortly.
Patch name: 0001-Set-a-more-informative-page-title-for-any-page
On Tue, Apr 13, 2021 at 8:51 PM Maxime Devos wrote:
>
> On Tue, 2021-04-13 at 18:56 +0300, Canan Talayhan wrote:
> > [...]
> > After sending the patch I've turned the patch like below.
> > (title ,(if title
> > `,(string-append title " - Guix Data Service")
> > '("Guix Data Service")))
>
> A little more simplification is possible:
>
> `,(string-append title ...) --> (string-append ...)
>
> The code `,A after macro-expansion simply becomes A, for every A.
> To test, you can run
> ,expand `,(string-append title " something")
> in a Guile REPL.
>
> Also,
> '("Guix Data Service")))
> seems incorrect. The end result if title is #f would be
> (title ("Guix Data Service")),
> while I believe you wanted
> (title "Guix Data Service").
>
> Correct code would be
>
> > (title ,(if title
> > `,(string-append title " - Guix Data Service")
> > "Guix Data Service"))
> or alternatively
>
> > (title ,(if title
> > `,(string-append title " - Guix Data Service")
> > '"Guix Data Service"))
> or
> > (title ,(if title
> > `,(string-append title " - Guix Data Service")
> > `"Guix Data Service"))
>
> I don't know how familiar you are with quasiquote (`), quote (') and
> unquote (,); it may be worthwhile to look this up in the Guile maual
>
> `,(string-append title " - Guix Data Service")
>
> I recommend sending e-mails as plain text. One benefit is that plain
> text doesn't mess up code indentation (at least on e-mail clients using
> fixed-width fonts for plain text, which seems standard).
>
> Greetings,
> Maxime.
commit f1a85cdf2beecdc3d01b1242088182e9a3a2af11
Author: Canan Talayhan
Date: Thu Apr 8 17:29:37 2021 +0300
Set a more informative page title for any page
where the title is "Guix Data Service"
---
guix-data-service/web/build/html.scm| 6 ++-
guix-data-service/web/dumps/html.scm| 6 ++-
guix-data-service/web/jobs/html.scm | 25 +--
guix-data-service/web/nar/html.scm | 6 ++-
guix-data-service/web/package/html.scm | 6 ++-
guix-data-service/web/revision/html.scm | 73 +++--
guix-data-service/web/view/html.scm | 12 --
7 files changed, 119 insertions(+), 15 deletions(-)
diff --git a/guix-data-service/web/build/html.scm b/guix-data-service/web/build/html.scm
index 18d045a..4b758bb 100644
--- a/guix-data-service/web/build/html.scm
+++ b/guix-data-service/web/build/html.scm
@@ -29,7 +29,11 @@
valid-targets
stats
builds)
+ (define page-header "Builds")
+
(layout
+ #:title
+ page-header
#:body
`(,(header)
(div
@@ -38,7 +42,7 @@
(@ (class "row"))
(div
(@ (class "col-sm-12"))
-(h1 "Builds")
+(h1 ,page-header)
(table
(@ (class "table"))
(thead
diff --git a/guix-data-service/web/dumps/html.scm b/guix-data-service/web/dumps/html.scm
index 71e69c8..c35cc1e 100644
--- a/guix-data-service/web/dumps/html.scm
+++ b/guix-data-service/web/dumps/html.scm
@@ -21,8 +21,12 @@
#:use-module (guix-data-service web view html)
#:export (view-dumps))
+(define page-header "Dumps")
+
(define (view-dumps available-dumps)
(layout
+ #:title
+ page-header
#:body
`(,(header)
(div
@@ -31,7 +35,7 @@
(@ (class "row"))
(div
(@ (class "col-sm-12"))
-(h1 "Database dumps")))
+(h1 ,page-header)))
,@(map
(match-lambda
((date-string . files)
diff --git a/guix-data-service/web/jobs/html.scm b/guix-data-service/web/jobs/html.scm
index 82734d6..54f6aaf 100644
--- a/guix-data-service/web/jobs/html.scm
+++ b/guix-data-service/web/jobs/html.scm
@@ -30,7 +30,11 @@
jobs-and-events
recent-events
show-next-page?)
+ (define page-header "Jobs")
+
(layout
+ #:title
+ page-header
#:body
`(,(header)
(div
@@ -40,7 +44,7 @@
(div
(@ (class "col-sm-12"))
(h1 (@ (style "display: inline-block;"))
-"Jobs")
+,page-header)
(div
(@ (class "btn-group pull-right")
(style "margin-top: 1.3rem;")
@@ -189,7 +193,11 @@
(define (view-job-events query-parameters
recent-events)
+ (define page-header "Recent Events")
+
(layout
+ #:title
+ page-header
#:body
`(,(header)
(div
@@ -200,7 +208,7 @@
(@ (class "col-sm-12"))
(a (@
Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
Luciana Lima Brito writes: > I implemented a basic json output for the derivation comparison page, > for my first contribution as an Outreachy applicant. > > The patch for the code I've changed is attached. > I'm waiting your reviews :) Hi Luciana, I'm not quite sure how to apply this, I'd suggest using git format-patch to generate the file next time as I think there would normally be some metadata along with the diff. Looking at the diff though: > diff --git a/guix-data-service/web/compare/controller.scm > b/guix-data-service/web/compare/controller.scm > index a6aa198..b7788cb 100644 > --- a/guix-data-service/web/compare/controller.scm > +++ b/guix-data-service/web/compare/controller.scm > @@ -584,19 +584,115 @@ >(derivation-differences-data conn > base-derivation > target-derivation) > - (case (most-appropriate-mime-type > - '(application/json text/html) > - mime-types) > -((application/json) > - (render-json > - '((error . "unimplemented")) ; TODO > - #:extra-headers http-headers-for-unchanging-content)) > -(else > - (render-html > - #:sxml (compare/derivation > - query-parameters > - data) > - #:extra-headers http-headers-for-unchanging-content))) > + (let ((outputs (assq-ref data 'outputs)) > +(inputs (assq-ref data 'inputs)) > +(sources (assq-ref data 'sources)) > +(system (assq-ref data 'system)) > +(builder (assq-ref data 'builder)) > +(args(assq-ref data 'arguments)) > +(environment-variables (assq-ref data > 'environment-variables)) > +(get-derivation-data > + (lambda (items) > + (map > +(match-lambda > + ((name path hash-alg hash recursive) > + `(,@(if (null? name) > + '() > + `((name . ,name))) > + ,@(if (null? path) > + '() > + `((path . ,path)) > + ) > + ,@(if (or (null? hash-alg) (not (string? hash-alg))) > + '() > + `((hash-algorithm . ,hash-alg)) > + ) > + ,@(if (or (null? hash) (not (string? hash))) > + '() > + `((hash . ,hash)) > + ) > + ,@(if (null? recursive) > + '() > + `((recursive . ,(string=? recursive "t")) > + ((derivation output) > + `(,@(if (null? derivation) > + '() > + `((derivation . ,derivation))) > + ,@(if (null? output) > + '() > + `((output . ,output) > + ((derivation) > + `(,@(if (null? derivation) > + '() > + `((derivation . ,derivation)) > +(or items '()) > + > +(let ((base-system (assq-ref system 'base)) > + (target-system (assq-ref system 'target)) > + (common-system (assq-ref system 'common)) > + > + (base-builder (assq-ref builder 'base)) > + (target-builder (assq-ref builder 'target)) > + (common-builder (assq-ref builder 'common)) > + > + (base-args (assq-ref args 'base)) > + (target-args (assq-ref args 'target)) > + (common-args (assq-ref args 'common))) > + > + (let ((matched-outputs (append-map get-derivation-data > + (list (assq-ref outputs > 'base) > + (assq-ref outputs > 'target) > + (assq-ref outputs > 'common > +(matched-inputs (append-map get-derivation-data > +(list (assq-ref inputs 'base) > + (assq-ref inputs > 'target > +(matched-sources (append-map get-derivation-data > + (list (assq-ref sources > 'base) > + (assq-ref sources > 'target)
