[Outreachy] - Guix Data Service - Set a more informative page title

[Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

[-- Attachment #1.1: Type: text/plain, Size: 985 bytes --]

Hi everyone,

My name is Canan. I'm an Outreachy applicant. I'm working on the
introductory task for Guix Data Service.


Introductory task:

   - Set a more informative page title for any page where the title is
   "Guix Data Service"


I've created a patch for the "Jobs" page. If it looks good for everyone
then I can proceed with other applicable pages.


Now, I'm working on the title part of the code snippet below to make it
more elegant.


```scm

(define* (layout #:key

                 (head '())

                 (body '())

                 title

                 description)

  `((doctype "html")

    (html

     (@ (lang "en"))

     (head

      ,@(if title

            `((title ,(string-append title " - Guix Data Service")))

            `((title "Guix Data Service")))

```


Could you please review and share your comments? I'll be appreciated.


Attached file: jobs-title-and-view.diff


Thanks,

Canan Talayhan

MSc. in Computer Engineering
Guix IRC: canant

[-- Attachment #1.2: Type: text/html, Size: 8155 bytes --]

[-- Attachment #2: jobs-title-and-view.diff --]
[-- Type: text/x-patch, Size: 3516 bytes --]

 guix-data-service/web/jobs/html.scm | 25 +++++++++++++++++++++----
 guix-data-service/web/view/html.scm | 11 +++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/guix-data-service/web/jobs/html.scm b/guix-data-service/web/jobs/html.scm
index 82734d6..44d4b68 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 (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Recent events")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -256,7 +264,11 @@
              recent-events)))))))))
 
 (define (view-job-queue jobs-and-events)
+  (define page-header "Queued Jobs")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -267,7 +279,7 @@
         (@ (class "col-sm-12"))
         (a (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Queued jobs ("
+        (h1 ,page-header"("
             ,(length jobs-and-events)
             ")")))
       (div
@@ -329,8 +341,13 @@
                              '())))))
                  jobs-and-events)))))))))
 
+
 (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)))
       (div
        (@ (class "row"))
        (div
diff --git a/guix-data-service/web/view/html.scm b/guix-data-service/web/view/html.scm
index 8063e17..94ea9a1 100644
--- a/guix-data-service/web/view/html.scm
+++ b/guix-data-service/web/view/html.scm
@@ -65,13 +65,15 @@
 (define* (layout #:key
                  (head '())
                  (body '())
-                 (title "Guix Data Service")
+                 title
                  description)
   `((doctype "html")
     (html
      (@ (lang "en"))
      (head
-      (title ,title)
+      ,@(if title
+            `((title ,(string-append title " - Guix Data Service")))
+            `((title "Guix Data Service")))
       (meta (@ (http-equiv "Content-Type")
                (content "text/html; charset=UTF-8")))
       (meta (@ (name "viewport")
@@ -286,8 +288,7 @@
 (define (index git-repositories-and-revisions)
   (layout
    #:description
-   "The Guix Data Service processes, stores and provides data about Guix over
-time."
+   "The Guix Data Service processes, stores and provides data about Guix over time."
    #:body
    `(,(header)
      (div
@@ -335,6 +336,8 @@ time."
 
 (define (view-statistics guix-revisions-count derivations-count)
   (layout
+   #:title
+   "Statistics"
    #:body
    `(,(header)
      (div

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]

On Tue, 2021-04-13 at 12:01 +0300, Canan Talayhan wrote:
> Hi everyone,
Welcome!

> My name is Canan. I'm an Outreachy applicant. I'm working on the introductory task for
> Guix Data Service.

Is this ‘introductory task’ publicly available?  If so, could you post a link?
I'm not up-to-date with Outreachy.

> Introductory task:
> Set a more informative page title for any page where the title is "Guix Data Service"
> 
> I've created a patch for the "Jobs" page. If it looks good for everyone then I can proceed with
>  other applicable pages.
> 
> Now, I'm working on the title part of the code snippet below to make it more elegant.
> 
> ```scm
> (define* (layout #:key
>                  (head '())
>                  (body '())
>                  title
>                  description)
>   `((doctype "html")
>     (html
>      (@ (lang "en"))
>      (head
>       ,@(if title
>             `((title ,(string-append title " - Guix Data Service")))

Typographical nitpick: I would use a proper dash here (the figure dash ‒ or the —
em dash, I always forgot which is correct), instead of the hyphen-minus -.
(Resource: <https://en.wikipedia.org/wiki/Dash#Figure_dash>).

>             `((title "Guix Data Service")))
> ```

You could bring the 'if' inside:
  `((title ,(if title
                (string-append title " ‒ Guix Data Service")
                "Guix Data Service")))


and a simplification is possible, as adding a title is unconditional:
  (head
    (title ,(if title (string-append ...) ...))
    ...)

> Could you please review and share your comments? I'll be appreciated.
> 
> Attached file: jobs-title-and-view.diff

+  (define page-header"Jobs")

Nitpick: I would put a space before "Jobs"?  Also, what's the point of
defining this variable, if it is constant?

+  (define page-header "Job")
+
   (layout
+   #:title
+   (string-append page-header job-id)

IIRC, job ids are integers, so this would result in titles
like "Job1234"?  Titles like "Job 1234" would be better.

I haven't tested the patch, but better page titles are good, thanks!

Maxime.
Sometimes reviewing patches, but without actually testing them.
Guix IRC: mdevos

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

[-- Attachment #1: Type: text/plain, Size: 3857 bytes --]

Hi Maxime,

Is this ‘introductory task’ publicly available?  If so, could you post a
> link?

I'm not up-to-date with Outreachy.
>

It's not available under the Official Outreachy page if you're not logged
in.


> Typographical nitpick: I would use a proper dash here (the figure dash ‒
> or the —
> em dash, I always forgot which is correct), instead of the hyphen-minus -.
> (Resource: <https://en.wikipedia.org/wiki/Dash#Figure_dash>).
>

It's a good catch : ) done.

You could bring the 'if' inside:
>   `((title ,(if title
>                 (string-append title " ‒ Guix Data Service")
>                 "Guix Data Service")))
>
>
> and a simplification is possible, as adding a title is unconditional:
>   (head
>     (title ,(if title (string-append ...) ...))
>     ...)
>

After sending the patch I've turned the patch like below.
      (title ,(if title
            `,(string-append title " - Guix Data Service")
            '("Guix Data Service")))

Nitpick: I would put a space before "Jobs"?  Also, what's the point of
> defining this variable, if it is constant?
>

We're generating titles using the available h1 value to prevent duplication.

Adding space between the id and job could be more readable, I agree. It's
done.

Thanks for the improvements,

Canan Talayhan

MSc. in Computer Engineering
Guix IRC: canant

On Tue, Apr 13, 2021 at 2:58 PM Maxime Devos <maximedevos@telenet.be> wrote:

> On Tue, 2021-04-13 at 12:01 +0300, Canan Talayhan wrote:
> > Hi everyone,
> Welcome!
>
> > My name is Canan. I'm an Outreachy applicant. I'm working on the
> introductory task for
> > Guix Data Service.
>
> Is this ‘introductory task’ publicly available?  If so, could you post a
> link?
> I'm not up-to-date with Outreachy.
>
> > Introductory task:
> > Set a more informative page title for any page where the title is "Guix
> Data Service"
> >
> > I've created a patch for the "Jobs" page. If it looks good for everyone
> then I can proceed with
> >  other applicable pages.
> >
> > Now, I'm working on the title part of the code snippet below to make it
> more elegant.
> >
> > ```scm
> > (define* (layout #:key
> >                  (head '())
> >                  (body '())
> >                  title
> >                  description)
> >   `((doctype "html")
> >     (html
> >      (@ (lang "en"))
> >      (head
> >       ,@(if title
> >             `((title ,(string-append title " - Guix Data Service")))
>
> Typographical nitpick: I would use a proper dash here (the figure dash ‒
> or the —
> em dash, I always forgot which is correct), instead of the hyphen-minus -.
> (Resource: <https://en.wikipedia.org/wiki/Dash#Figure_dash>).
>
> >             `((title "Guix Data Service")))
> > ```
>
> You could bring the 'if' inside:
>   `((title ,(if title
>                 (string-append title " ‒ Guix Data Service")
>                 "Guix Data Service")))
>
>
> and a simplification is possible, as adding a title is unconditional:
>   (head
>     (title ,(if title (string-append ...) ...))
>     ...)
>
> > Could you please review and share your comments? I'll be appreciated.
> >
> > Attached file: jobs-title-and-view.diff
>
> +  (define page-header"Jobs")
>
> Nitpick: I would put a space before "Jobs"?  Also, what's the point of
> defining this variable, if it is constant?
>
> +  (define page-header "Job")
> +
>    (layout
> +   #:title
> +   (string-append page-header job-id)
>
> IIRC, job ids are integers, so this would result in titles
> like "Job1234"?  Titles like "Job 1234" would be better.
>
> I haven't tested the patch, but better page titles are good, thanks!
>
> Maxime.
> Sometimes reviewing patches, but without actually testing them.
> Guix IRC: mdevos
>

[-- Attachment #2: Type: text/html, Size: 6847 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

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 <maximedevos@telenet.be> 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.

[-- Attachment #2: 0001-Set-a-more-informative-page-title-for-any-page.patch --]
[-- Type: text/x-patch, Size: 13684 bytes --]

commit f1a85cdf2beecdc3d01b1242088182e9a3a2af11
Author: Canan Talayhan <canan.t.talayhan@gmail.com>
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 (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Recent events")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -256,7 +264,11 @@
              recent-events)))))))))
 
 (define (view-job-queue jobs-and-events)
+  (define page-header "Queued Jobs")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -267,7 +279,7 @@
         (@ (class "col-sm-12"))
         (a (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Queued jobs ("
+        (h1 ,page-header"("
             ,(length jobs-and-events)
             ")")))
       (div
@@ -329,8 +341,13 @@
                              '())))))
                  jobs-and-events)))))))))
 
+
 (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)))
       (div
        (@ (class "row"))
        (div
diff --git a/guix-data-service/web/nar/html.scm b/guix-data-service/web/nar/html.scm
index 596d16b..07e6c47 100644
--- a/guix-data-service/web/nar/html.scm
+++ b/guix-data-service/web/nar/html.scm
@@ -21,8 +21,12 @@
   #:use-module (guix-data-service web view html)
   #:export (view-substitutes))
 
+(define page-header "Substitutes")
+
 (define (view-substitutes narinfo-signing-public-key)
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -31,7 +35,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Substitutes")
+        (h1 ,page-header)
         ,@(if (canonical-sexp? narinfo-signing-public-key)
               `((h3 "Public key")
                 (pre
diff --git a/guix-data-service/web/package/html.scm b/guix-data-service/web/package/html.scm
index 0d9b078..af4552e 100644
--- a/guix-data-service/web/package/html.scm
+++ b/guix-data-service/web/package/html.scm
@@ -24,7 +24,11 @@
   #:export (view-package))
 
 (define* (view-package name package-version-with-branches)
+  (define page-header "Package")
+
   (layout
+   #:title
+   (string-append page-header " " name)
    #:body
    `(,(header)
      (div
@@ -33,7 +37,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-md-12"))
-        (h1 "Package: " ,name)))
+        (h1 ,page-header ," " ,name)))
       ,@(map
          (match-lambda
            ((('version . version)
diff --git a/guix-data-service/web/revision/html.scm b/guix-data-service/web/revision/html.scm
index 25b79f4..0685523 100644
--- a/guix-data-service/web/revision/html.scm
+++ b/guix-data-service/web/revision/html.scm
@@ -48,7 +48,11 @@
 (define* (view-revision-news commit-hash
                              query-parameters
                              news-entries)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take commit-hash 7) " Channel News Entries")
    #:body
    `(,(header)
      (div
@@ -59,7 +63,7 @@
         (@ (class "col-sm-12"))
         (h3 (a (@ (style "white-space: nowrap;")
                   (href ,(string-append "/revision/" commit-hash)))
-               "Revision " (samp ,commit-hash)))))
+               ,page-header " " (samp ,commit-hash)))))
       (div
        (@ (class "row"))
        (div
@@ -107,7 +111,11 @@
                                 #:key path-base
                                 header-text
                                 header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take(string-drop header-link 10)7) " Package " name)
    #:body
    `(,(header)
      (div
@@ -135,7 +143,7 @@
                                          branch-name))))
                   branches)))
           git-repositories-and-branches)
-        (h1 "Package " ,name)))
+        (h1 ,"Package " ,name)))
       (div
        (@ (class "row"))
        (div
@@ -169,7 +177,11 @@
                                             #:key header-text
                                             header-link
                                             version-history-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take(string-drop header-link 10)7) " Package" " " name " @" version)
    #:body
    `(,(header)
      (div
@@ -224,7 +236,7 @@
                       (role "button"))
                    "Version history"))
               '())
-        (h1 "Package " ,name " @ " ,version)))
+        (h1 ,page-header ," " ,name " @ " ,version)))
       (div
        (@ (class "row"))
        (div
@@ -471,7 +483,11 @@
                         lint-warning-counts
                         #:key (path-base "/revision/")
                         header-text)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -547,7 +563,11 @@
      '("Version" "Synopsis" "Description"
        "Home page" "Location" "Licenses")))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Packages")
    #:body
    `(,(header)
      (div
@@ -755,7 +775,11 @@
                    total-package-descriptions)))))
      package-description-counts))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Packages Translation Availability")
    #:body
    `(,(header)
      (div
@@ -835,7 +859,11 @@
                                      query-parameters
                                      #:key (path-base "/revision/")
                                      header-text header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " System Tests")
    #:body
    `(,(header)
      (div
@@ -936,7 +964,11 @@
                                           channel-instances
                                           #:key (path-base "/revision/")
                                           header-text header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Channel Instances")
    #:body
    `(,(header)
      (div
@@ -1216,7 +1248,11 @@ figure {
                 data-percentages
                 colours))))))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take revision-commit-hash 7) " Package Substitute Availability")
    #:body
    `(,(header)
      (style ,chart-css)
@@ -1254,7 +1290,11 @@ figure {
                                                 #:key (path-base "/revision/")
                                                 header-text
                                                 header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Package Reproducibility")
    #:body
    `(,(header)
      (style "
@@ -1521,7 +1561,11 @@ figure {
   (define fields
     (assq-ref query-parameters 'field))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Package Derivations")
    #:body
    `(,(header)
      (div
@@ -1702,7 +1746,11 @@ figure {
       ;;("Unknown"   . "unknown") TODO
       ))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Fixed Output Package Derivations")
    #:body
    `(,(header)
      (div
@@ -1842,7 +1890,11 @@ figure {
             (cons url id)))
          build-server-urls))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Package Derivation Outputs")
    #:body
    `(,(header)
      (div
@@ -2021,7 +2073,11 @@ figure {
                               build-server-options
                               stats
                               builds)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take commit-hash 7) " Builds")
    #:body
    `(,(header)
      (div
@@ -2158,7 +2214,11 @@ figure {
               (string-downcase field))))
      '("Linter" "Message" "Location")))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take (string-drop header-link 10) 7) " Lint Warnings")
    #:body
    `(,(header)
      (div
@@ -2314,7 +2374,11 @@ figure {
 
 (define (unknown-revision commit-hash job git-repositories-and-branches
                           jobs-and-events)
+  (define page-header "Unknown Revision")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2353,7 +2417,10 @@ figure {
 
 (define (unprocessed-revision commit-hash job git-repositories-and-branches
                               jobs-and-events)
+  (define page-header "Unprocessed Revision")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
diff --git a/guix-data-service/web/view/html.scm b/guix-data-service/web/view/html.scm
index 8063e17..33535a9 100644
--- a/guix-data-service/web/view/html.scm
+++ b/guix-data-service/web/view/html.scm
@@ -65,13 +65,15 @@
 (define* (layout #:key
                  (head '())
                  (body '())
-                 (title "Guix Data Service")
+                 title
                  description)
   `((doctype "html")
     (html
      (@ (lang "en"))
      (head
-      (title ,title)
+      (title ,(if title
+               (string-append title " — Guix Data Service")
+               "Guix Data Service"))
       (meta (@ (http-equiv "Content-Type")
                (content "text/html; charset=UTF-8")))
       (meta (@ (name "viewport")
@@ -286,8 +288,7 @@
 (define (index git-repositories-and-revisions)
   (layout
    #:description
-   "The Guix Data Service processes, stores and provides data about Guix over
-time."
+   "The Guix Data Service processes, stores and provides data about Guix over time."
    #:body
    `(,(header)
      (div
@@ -334,7 +335,10 @@ time."
          git-repositories-and-revisions)))))
 
 (define (view-statistics guix-revisions-count derivations-count)
+  (define page-header "Statistics")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]


Canan Talayhan <canan.t.talayhan@gmail.com> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

Hi again,

@Chris
After your suggestions, I've fixed the patch. But for the revision
part, I have a question. You said "I'd put "Channel News Entries"
first, then the Revision bit second." There are so many titles like
this structure in Revision views.

For example :
->System tests Revision c7d0441
or
->Revision c7d0441 System tests

Should I fix all of them this way? Or just for Channel News Entries?

I've set the rule for 80 chars. Thanks. :)

On Fri, Apr 16, 2021 at 12:52 AM Christopher Baines <mail@cbaines.net> wrote:
>
>
> Canan Talayhan <canan.t.talayhan@gmail.com> 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.

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]


Canan Talayhan <canan.t.talayhan@gmail.com> writes:

> After your suggestions, I've fixed the patch. But for the revision
> part, I have a question. You said "I'd put "Channel News Entries"
> first, then the Revision bit second." There are so many titles like
> this structure in Revision views.
>
> For example :
> ->System tests Revision c7d0441
> or
> ->Revision c7d0441 System tests
>
> Should I fix all of them this way? Or just for Channel News Entries?

I think having the System tests bit first is better, that way different
pages about the same revision will have more distinct titles, and yeah,
I'd apply the same principle to other titles as well.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

[-- Attachment #1.1: Type: text/plain, Size: 1519 bytes --]

Hi Chris,

I've updated the patch that contains all the suggestions. I think the patch
is ready to merge.

One thing that I would like to ask you about the package and package-name
in web/repository/controller.scm.

When I test the URL below I'm getting this error. (
https://pastebin.ubuntu.com/p/HdKShmKqH7/)

   - ('GET "repository" repository-id "branch" branch-name "package"
   package-name) ->
   http://localhost:8765/repository/1/branch/master/package/emacs

What do you think? BTW it's accessible on the official server.

   - https://data.guix.gnu.org/repository/1/branch/master/package/emacs/

Could you please review the patch attached?
I'm very excited to make my first FOSS contribution. :)

Thanks for your help,
Canan Talayhan


On Fri, Apr 16, 2021 at 2:11 PM Christopher Baines <mail@cbaines.net> wrote:

>
> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
>
> > After your suggestions, I've fixed the patch. But for the revision
> > part, I have a question. You said "I'd put "Channel News Entries"
> > first, then the Revision bit second." There are so many titles like
> > this structure in Revision views.
> >
> > For example :
> > ->System tests Revision c7d0441
> > or
> > ->Revision c7d0441 System tests
> >
> > Should I fix all of them this way? Or just for Channel News Entries?
>
> I think having the System tests bit first is better, that way different
> pages about the same revision will have more distinct titles, and yeah,
> I'd apply the same principle to other titles as well.
>

[-- Attachment #1.2: Type: text/html, Size: 2327 bytes --]

[-- Attachment #2: 0001-Set-a-more-informative-page-title-for-any-page.patch --]
[-- Type: text/x-patch, Size: 22711 bytes --]

From f3d02bf0e7050f544e044b8dd53d52e9dba28d54 Mon Sep 17 00:00:00 2001
From: Canan Talayhan <canan.t.talayhan@gmail.com>
Date: Sun, 18 Apr 2021 14:50:20 +0300
Subject: [PATCH] Set a more informative page title for any page where the
 title is "Guix Data Service"

---
 guix-data-service/web/build-server/html.scm | 24 +++++-
 guix-data-service/web/build/html.scm        |  6 +-
 guix-data-service/web/compare/html.scm      | 32 ++++++--
 guix-data-service/web/dumps/html.scm        |  7 +-
 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/repository/html.scm   | 30 ++++++-
 guix-data-service/web/revision/html.scm     | 89 +++++++++++++++++++--
 guix-data-service/web/view/html.scm         | 12 ++-
 10 files changed, 209 insertions(+), 28 deletions(-)

diff --git a/guix-data-service/web/build-server/html.scm b/guix-data-service/web/build-server/html.scm
index f16a570..541a960 100644
--- a/guix-data-service/web/build-server/html.scm
+++ b/guix-data-service/web/build-server/html.scm
@@ -27,7 +27,11 @@
 (define (view-build query-parameters
                     build
                     required-failed-builds)
+  (define page-header "Build")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -36,7 +40,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Build")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        ,@(match build
@@ -98,7 +102,11 @@
             '())))))
 
 (define (view-build-servers build-servers)
+  (define page-header "Build servers")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -107,7 +115,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build servers")
+        (h2 ,page-header)
         ,@(map
            (match-lambda
              ((id url lookup-all-derivations? lookup-builds?)
@@ -127,7 +135,11 @@
            build-servers)))))))
 
 (define (view-build-server build-server)
+  (define page-header "Build server")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -136,7 +148,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build server")
+        (h2 ,page-header)
         ,(match build-server
            ((url lookup-all-derivations?)
             `(dl
@@ -150,7 +162,11 @@
                        "No")))))))))))
 
 (define (view-signing-key sexp)
+  (define page-header "Signing key")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -159,5 +175,5 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Signing key")
+        (h2 ,page-header)
         ,(sexp-div sexp)))))))
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/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..170fb12 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -96,7 +96,12 @@
     (unless invalid-query?
       (query-parameters->string query-parameters)))
 
+  (define page-header "Comparing")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take base-commit 8) " and "
+    (string-take target-commit 8))
    #:body
    `(,(header)
      (div
@@ -107,7 +112,7 @@
         (@ (class "col-sm-7"))
         ,@(if invalid-query?
               `((h1 "Compare"))
-              `((h1 "Comparing "
+              `((h1 ,page-header ," "
                     (a (@ (href ,(string-append "/revision/" base-commit)))
                        (samp ,(string-take base-commit 8) "…"))
                     " and "
@@ -419,14 +424,18 @@
     '(span (@ (class "text-success glyphicon glyphicon-plus pull-left")
               (style "font-size: 1.5em; padding-right: 0.4em;"))))
 
+  (define page-header "Comparing derivations")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
       (@ (class "container"))
       (div
        (@ (class "row"))
-       (h1 ,@(let ((base-commit (assq-ref query-parameters 'base_commit))
+       (h1 ,(let ((base-commit (assq-ref query-parameters 'base_commit))
                    (target-commit (assq-ref query-parameters 'target_commit)))
                (if (every string? (list base-commit target-commit))
                    `("Comparing "
@@ -435,7 +444,7 @@
                      " and "
                      (a (@ (href ,(string-append "/revision/" target-commit)))
                         (samp ,(string-take target-commit 8) "…")))
-                   '("Comparing derivations")))))
+                    `,page-header))))
       (div
        (@ (class "row"))
        (div
@@ -685,7 +694,11 @@
   (define fields
     (assq-ref query-parameters 'field))
 
+  (define page-header "Package derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -835,7 +848,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Package derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? derivation-changes)
           '(p "No derivation changes")
@@ -950,7 +963,12 @@ enough builds to determine a change")))
     (string-append "?base_commit=" base-commit
                    "&target_commit=" target-commit))
 
+  (define page-header (string-append "Comparing "
+    (string-take base-commit 8) " and " (string-take target-commit 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1042,7 +1060,11 @@ enough builds to determine a change")))
                                           #:optional
                                           base-revision-details
                                           target-revision-details)
+  (define page-header "System test derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1141,7 +1163,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "System test derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? changes)
           '(p "No system test derivation changes")
diff --git a/guix-data-service/web/dumps/html.scm b/guix-data-service/web/dumps/html.scm
index 71e69c8..9645f7c 100644
--- a/guix-data-service/web/dumps/html.scm
+++ b/guix-data-service/web/dumps/html.scm
@@ -21,8 +21,13 @@
   #:use-module (guix-data-service web view html)
   #:export (view-dumps))
 
+(define page-header "Database dumps")
+
 (define (view-dumps available-dumps)
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -31,7 +36,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..7165e8d 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 (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Recent events")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -256,7 +264,11 @@
              recent-events)))))))))
 
 (define (view-job-queue jobs-and-events)
+  (define page-header "Queued Jobs")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -267,7 +279,7 @@
         (@ (class "col-sm-12"))
         (a (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Queued jobs ("
+        (h1 ,page-header"("
             ,(length jobs-and-events)
             ")")))
       (div
@@ -329,8 +341,13 @@
                              '())))))
                  jobs-and-events)))))))))
 
+
 (define (view-job job-id query-parameters log)
+  (define page-header (string-append "Job " job-id))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -339,7 +356,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Job " ,job-id)))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
diff --git a/guix-data-service/web/nar/html.scm b/guix-data-service/web/nar/html.scm
index 596d16b..07e6c47 100644
--- a/guix-data-service/web/nar/html.scm
+++ b/guix-data-service/web/nar/html.scm
@@ -21,8 +21,12 @@
   #:use-module (guix-data-service web view html)
   #:export (view-substitutes))
 
+(define page-header "Substitutes")
+
 (define (view-substitutes narinfo-signing-public-key)
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -31,7 +35,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Substitutes")
+        (h1 ,page-header)
         ,@(if (canonical-sexp? narinfo-signing-public-key)
               `((h3 "Public key")
                 (pre
diff --git a/guix-data-service/web/package/html.scm b/guix-data-service/web/package/html.scm
index 0d9b078..af4552e 100644
--- a/guix-data-service/web/package/html.scm
+++ b/guix-data-service/web/package/html.scm
@@ -24,7 +24,11 @@
   #:export (view-package))
 
 (define* (view-package name package-version-with-branches)
+  (define page-header "Package")
+
   (layout
+   #:title
+   (string-append page-header " " name)
    #:body
    `(,(header)
      (div
@@ -33,7 +37,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-md-12"))
-        (h1 "Package: " ,name)))
+        (h1 ,page-header ," " ,name)))
       ,@(map
          (match-lambda
            ((('version . version)
diff --git a/guix-data-service/web/repository/html.scm b/guix-data-service/web/repository/html.scm
index 88f2632..5b8e220 100644
--- a/guix-data-service/web/repository/html.scm
+++ b/guix-data-service/web/repository/html.scm
@@ -32,7 +32,11 @@
             view-no-latest-revision))
 
 (define* (view-git-repositories git-repositories)
+   (define page-header "Git repositories")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -41,7 +45,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-md-12"))
-        (h1 "Git repositories")))
+        (h1 ,page-header)))
       ,@(map
          (match-lambda
            ((id label url cgit-base-url)
@@ -65,7 +69,11 @@
 (define* (view-git-repository git-repository-id
                               label url cgit-url-base
                               branches-with-most-recent-commits)
+  (define page-header (string-append "Repository " (string-drop url 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -86,7 +94,11 @@
 
 (define (view-branch git-repository-id
                      branch-name query-parameters branch-commits)
+  (define page-header (string-append branch-name " branch"))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -197,7 +209,11 @@
                              branch-name
                              package-name
                              versions-by-revision-range)
+  (define page-header (string-append branch-name " " package-name))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -386,6 +402,8 @@
                 (map first derivations-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " Package derivations")
    #:body
    `(,(header)
      (div
@@ -636,6 +654,8 @@
                 (map first outputs-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " Package outputs")
    #:body
    `(,(header)
      (div
@@ -849,6 +869,8 @@
                                          valid-systems
                                          system-test-history)
   (layout
+   #:title
+   (string-append system-test-name " History")
    #:body
    `(,(header)
      (div
@@ -1016,12 +1038,16 @@
                 '(#f))))))))))))
 
 (define (view-no-latest-revision branch-name)
+   (define page-header "No latest revision")
+
   (layout
+   #:title
+   (string-append page-header " for " branch-name)
    #:body
    `(,(header)
      (div
       (@ (class "container"))
-      (h1 "No latest revision")
+      (h1 ,page-header)
       (p "No latest revision for "
          (strong (samp ,branch-name))
          " branch")))))
diff --git a/guix-data-service/web/revision/html.scm b/guix-data-service/web/revision/html.scm
index 25b79f4..ab08eaa 100644
--- a/guix-data-service/web/revision/html.scm
+++ b/guix-data-service/web/revision/html.scm
@@ -48,7 +48,12 @@
 (define* (view-revision-news commit-hash
                              query-parameters
                              news-entries)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Channel News Entries " page-header " "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -59,7 +64,7 @@
         (@ (class "col-sm-12"))
         (h3 (a (@ (style "white-space: nowrap;")
                   (href ,(string-append "/revision/" commit-hash)))
-               "Revision " (samp ,commit-hash)))))
+               ,page-header " " (samp ,commit-hash)))))
       (div
        (@ (class "row"))
        (div
@@ -107,7 +112,12 @@
                                 #:key path-base
                                 header-text
                                 header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Package " name " " page-header " "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -135,7 +145,7 @@
                                          branch-name))))
                   branches)))
           git-repositories-and-branches)
-        (h1 "Package " ,name)))
+        (h1 ,"Package " ,name)))
       (div
        (@ (class "row"))
        (div
@@ -169,7 +179,12 @@
                                             #:key header-text
                                             header-link
                                             version-history-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Package " name " @ " version " " page-header " "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -224,7 +239,7 @@
                       (role "button"))
                    "Version history"))
               '())
-        (h1 "Package " ,name " @ " ,version)))
+        (h1 ,"Package " ,name " @ " ,version)))
       (div
        (@ (class "row"))
        (div
@@ -471,7 +486,11 @@
                         lint-warning-counts
                         #:key (path-base "/revision/")
                         header-text)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append page-header " " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -547,7 +566,12 @@
      '("Version" "Synopsis" "Description"
        "Home page" "Location" "Licenses")))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append  "Packages " page-header " "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -755,7 +779,12 @@
                    total-package-descriptions)))))
      package-description-counts))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Packages translation availability " page-header " "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -835,7 +864,11 @@
                                      query-parameters
                                      #:key (path-base "/revision/")
                                      header-text header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append  "System tests " page-header " " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -936,7 +969,12 @@
                                           channel-instances
                                           #:key (path-base "/revision/")
                                           header-text header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Channel instances " page-header " "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1216,7 +1254,12 @@ figure {
                 data-percentages
                 colours))))))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Package substitute availability " page-header " "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style ,chart-css)
@@ -1254,7 +1297,12 @@ figure {
                                                 #:key (path-base "/revision/")
                                                 header-text
                                                 header-link)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append  "Package reproducibility " page-header " "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style "
@@ -1521,7 +1569,12 @@ figure {
   (define fields
     (assq-ref query-parameters 'field))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append  "Package derivations " page-header " "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1702,7 +1755,12 @@ figure {
       ;;("Unknown"   . "unknown") TODO
       ))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append  "Fixed output package derivations " page-header " "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1842,7 +1900,12 @@ figure {
             (cons url id)))
          build-server-urls))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Package derivation outputs " page-header " "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2021,7 +2084,11 @@ figure {
                               build-server-options
                               stats
                               builds)
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append  "Builds " page-header " " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2158,7 +2225,12 @@ figure {
               (string-downcase field))))
      '("Linter" "Message" "Location")))
 
+  (define page-header "Revision")
+
   (layout
+   #:title
+   (string-append "Lint warnings " page-header " "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2314,7 +2386,11 @@ figure {
 
 (define (unknown-revision commit-hash job git-repositories-and-branches
                           jobs-and-events)
+  (define page-header "Uknown revision")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2347,13 +2423,16 @@ figure {
                 (strong (@ (class "text-center")
                            (style "font-size: 2em; display: block;"))
                         "Unknown"))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
 
 (define (unprocessed-revision commit-hash job git-repositories-and-branches
                               jobs-and-events)
+  (define page-header "Unknown revision")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2375,6 +2454,6 @@ figure {
                        git-repositories-and-branches
                        commit-hash))
                 ,@(view-revision/jobs-and-events jobs-and-events))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
diff --git a/guix-data-service/web/view/html.scm b/guix-data-service/web/view/html.scm
index 8063e17..33535a9 100644
--- a/guix-data-service/web/view/html.scm
+++ b/guix-data-service/web/view/html.scm
@@ -65,13 +65,15 @@
 (define* (layout #:key
                  (head '())
                  (body '())
-                 (title "Guix Data Service")
+                 title
                  description)
   `((doctype "html")
     (html
      (@ (lang "en"))
      (head
-      (title ,title)
+      (title ,(if title
+               (string-append title " — Guix Data Service")
+               "Guix Data Service"))
       (meta (@ (http-equiv "Content-Type")
                (content "text/html; charset=UTF-8")))
       (meta (@ (name "viewport")
@@ -286,8 +288,7 @@
 (define (index git-repositories-and-revisions)
   (layout
    #:description
-   "The Guix Data Service processes, stores and provides data about Guix over
-time."
+   "The Guix Data Service processes, stores and provides data about Guix over time."
    #:body
    `(,(header)
      (div
@@ -334,7 +335,10 @@ time."
          git-repositories-and-revisions)))))
 
 (define (view-statistics guix-revisions-count derivations-count)
+  (define page-header "Statistics")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
-- 
2.17.1

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 9439 bytes --]


Canan Talayhan <canan.t.talayhan@gmail.com> writes:

> I've updated the patch that contains all the suggestions. I think the patch
> is ready to merge.
>
> One thing that I would like to ask you about the package and package-name
> in web/repository/controller.scm.
>
> When I test the URL below I'm getting this error. (
> https://pastebin.ubuntu.com/p/HdKShmKqH7/)
>
>    - ('GET "repository" repository-id "branch" branch-name "package"
>    package-name) ->
>    http://localhost:8765/repository/1/branch/master/package/emacs
>
> What do you think? BTW it's accessible on the official server.
>
>    - https://data.guix.gnu.org/repository/1/branch/master/package/emacs/

Hmm, this could possibly be due to an issue with the small dump of the
database.

> Could you please review the patch attached?
> I'm very excited to make my first FOSS contribution. :)

I've had a look though, and I have some more comments:

  diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
  index 5b5fe0a..170fb12 100644
  --- a/guix-data-service/web/compare/html.scm
  +++ b/guix-data-service/web/compare/html.scm
  @@ -96,7 +96,12 @@
       (unless invalid-query?
         (query-parameters->string query-parameters)))

  +  (define page-header "Comparing")
  +
     (layout
  +   #:title
  +   (string-append page-header " " (string-take base-commit 8) " and "
  +    (string-take target-commit 8))
      #:body
      `(,(header)
        (div
  @@ -107,7 +112,7 @@
           (@ (class "col-sm-7"))
           ,@(if invalid-query?
                 `((h1 "Compare"))
  -              `((h1 "Comparing "
  +              `((h1 ,page-header ," "
                       (a (@ (href ,(string-append "/revision/" base-commit)))
                          (samp ,(string-take base-commit 8) "…"))
                       " and "

There's a couple of things here. I'd be tempted not to use a variable
for "Comparing", it's not really the page header, as that's more
complicated, so I think I'd just use the string in both places.

Second thing, the (if invalid-query? bit when constructing the h1
element is important. The query parameters being invalid could mean
anything from the form just hasn't been filled in, to the value isn't
actually a commit hash, but something else, maybe some HTML/JavaScript
that is malicious and shouldn't be included in the page. A similar
approach probably needs taking for the title.

  @@ -419,14 +424,18 @@
       '(span (@ (class "text-success glyphicon glyphicon-plus pull-left")
                 (style "font-size: 1.5em; padding-right: 0.4em;"))))

  +  (define page-header "Comparing derivations")
  +
     (layout
  +   #:title
  +   page-header
      #:body
      `(,(header)
        (div
         (@ (class "container"))
         (div
          (@ (class "row"))
  -       (h1 ,@(let ((base-commit (assq-ref query-parameters 'base_commit))
  +       (h1 ,(let ((base-commit (assq-ref query-parameters  'base_commit))

Why's the @ being removed here?

  @@ -435,7 +444,7 @@
                        " and "
                        (a (@ (href ,(string-append "/revision/" target-commit)))
                           (samp ,(string-take target-commit 8) "…")))
  -                   '("Comparing derivations")))))
  +                    `,page-header))))

The quote then immediate unquote here isn't necessary, also, I think
this should stick to being a list containing a string, as the other part
of the if returns a list.

  diff --git a/guix-data-service/web/dumps/html.scm b/guix-data-service/web/dumps/html.scm
  index 71e69c8..9645f7c 100644
  --- a/guix-data-service/web/dumps/html.scm
  +++ b/guix-data-service/web/dumps/html.scm
  @@ -21,8 +21,13 @@
     #:use-module (guix-data-service web view html)
     #:export (view-dumps))

  +(define page-header "Database dumps")
  +
   (define (view-dumps available-dumps)
  +
     (layout
  +   #:title
  +   page-header
      #:body
      `(,(header)
        (div
  @@ -31,7 +36,7 @@
          (@ (class "row"))
          (div
           (@ (class "col-sm-12"))
  -        (h1 "Database dumps")))
  +        (h1 ,page-header)))

Like the others, I'd probably put page-header inside the view-dumps
procedure. Same goes for other places where it's outside.

  @@ -267,7 +279,7 @@
           (@ (class "col-sm-12"))
           (a (@ (href "/jobs"))
              (h3 "Jobs"))
  -        (h1 "Queued jobs ("
  +        (h1 ,page-header"("
               ,(length jobs-and-events)
               ")")))
         (div

I'd suspect the title here would be "Queued jobs(", I'd also put a space
between ,page-header the bit after it in the code.

  @@ -329,8 +341,13 @@
                                '())))))
                    jobs-and-events)))))))))

  +
   (define (view-job job-id query-parameters log)
  +  (define page-header (string-append "Job " job-id))
  +
     (layout
  +   #:title
  +   page-header
      #:body
      `(,(header)
        (div

Most of the procedures are separated by one line, and I wouldn't change
that here.

  @@ -24,7 +24,11 @@
     #:export (view-package))

   (define* (view-package name package-version-with-branches)
  +  (define page-header "Package")
  +
     (layout
  +   #:title
  +   (string-append page-header " " name)
      #:body
      `(,(header)
        (div
  @@ -33,7 +37,7 @@
          (@ (class "row"))
          (div
           (@ (class "col-md-12"))
  -        (h1 "Package: " ,name)))
  +        (h1 ,page-header ," " ,name)))
         ,@(map
            (match-lambda
              ((('version . version)

I'm not that fussed about the colon, but I'd probably keep it.

I'd try to keep the page-header variable meaningful if you're going to
use it though. "Package" is named as the page-header, but it's not what
the title is, or the h1 element. They're both better as they include the
package name, I'd probably just make the page-header the actual string
used in both places.

  @@ -65,7 +69,11 @@
   (define* (view-git-repository git-repository-id
                                 label url cgit-url-base
                                 branches-with-most-recent-commits)
  +  (define page-header (string-append "Repository " (string-drop url 8)))
  +
     (layout
  +   #:title
  +   page-header
      #:body
      `(,(header)
        (div

This is really nice, it's good that the pages for different repositories
won't have the same title.

  @@ -197,7 +209,11 @@
                                branch-name
                                package-name
                                versions-by-revision-range)
  +  (define page-header (string-append branch-name " " package-name))
  +
     (layout
  +   #:title
  +   page-header
      #:body
      `(,(header)
        (div

I think you might need some more words for this title to make sense
though, even just having package-name " on " branch-name would probably
help people work out what's going on.

  @@ -386,6 +402,8 @@
                   (map first derivations-by-revision-range))))

     (layout
  +   #:title
  +   (string-append package-name " Package derivations")
      #:body
      `(,(header)
        (div
  @@ -636,6 +654,8 @@
                   (map first outputs-by-revision-range))))

     (layout
  +   #:title
  +   (string-append package-name " Package outputs")
      #:body
      `(,(header)
        (div
  @@ -849,6 +869,8 @@
                                            valid-systems
                                            system-test-history)
     (layout
  +   #:title
  +   (string-append system-test-name " History")
      #:body
      `(,(header)
        (div

Given there's something coming first in these titles, I wouldn't
capitalise the H in History or P in Package, since it's not at the start
of the title.

  @@ -48,7 +48,12 @@
   (define* (view-revision-news commit-hash
                                query-parameters
                                news-entries)
  +  (define page-header "Revision")
  +
     (layout
  +   #:title
  +   (string-append "Channel News Entries " page-header " "
  +    (string-take commit-hash 7))
      #:body
      `(,(header)
        (div

  @@ -48,7 +48,12 @@
   (define* (view-revision-news commit-hash
                                query-parameters
                                news-entries)
  +  (define page-header "Revision")
  +
     (layout
  +   #:title
  +   (string-append "Channel News Entries " page-header " "
  +    (string-take commit-hash 7))
      #:body
      `(,(header)
        (div

Same thing here regarding defining a page-header which isn't actually
used as the header.

I do think it's useful to include both the Channel News Entries bit and
the revision bit in the title, but I'd probably put a separator (like a
-) in between them. Same goes for the other changes in this file.

  @@ -2314,7 +2386,11 @@ figure {

   (define (unknown-revision commit-hash job git-repositories-and-branches
                             jobs-and-events)
  +  (define page-header "Uknown revision")
  +
     (layout
  +   #:title
  +   page-header
      #:body
      `(,(header)
        (div

A letter was lost here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

[-- Attachment #1: Type: text/plain, Size: 10624 bytes --]

Thanks for your quick response.

>Why's the @ being removed here?
It interprets like an HTML code when I use the page-header like
`,page-header, so I removed it. According to your comment, I reverted
to the original version.

" 'GET repository..." which includes package/package-name in the URL
has not the best titles since I couldn't test them because of the
error that I've mentioned.
I'm open to suggestions.

Could you please re-review the patch that contains all the
modifications you've mentioned in the previous message?

On Sun, Apr 18, 2021 at 8:53 PM Christopher Baines <mail@cbaines.net> wrote:
>
>
> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
>
> > I've updated the patch that contains all the suggestions. I think the patch
> > is ready to merge.
> >
> > One thing that I would like to ask you about the package and package-name
> > in web/repository/controller.scm.
> >
> > When I test the URL below I'm getting this error. (
> > https://pastebin.ubuntu.com/p/HdKShmKqH7/)
> >
> >    - ('GET "repository" repository-id "branch" branch-name "package"
> >    package-name) ->
> >    http://localhost:8765/repository/1/branch/master/package/emacs
> >
> > What do you think? BTW it's accessible on the official server.
> >
> >    - https://data.guix.gnu.org/repository/1/branch/master/package/emacs/
>
> Hmm, this could possibly be due to an issue with the small dump of the
> database.
>
> > Could you please review the patch attached?
> > I'm very excited to make my first FOSS contribution. :)
>
> I've had a look though, and I have some more comments:
>
>   diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
>   index 5b5fe0a..170fb12 100644
>   --- a/guix-data-service/web/compare/html.scm
>   +++ b/guix-data-service/web/compare/html.scm
>   @@ -96,7 +96,12 @@
>        (unless invalid-query?
>          (query-parameters->string query-parameters)))
>
>   +  (define page-header "Comparing")
>   +
>      (layout
>   +   #:title
>   +   (string-append page-header " " (string-take base-commit 8) " and "
>   +    (string-take target-commit 8))
>       #:body
>       `(,(header)
>         (div
>   @@ -107,7 +112,7 @@
>            (@ (class "col-sm-7"))
>            ,@(if invalid-query?
>                  `((h1 "Compare"))
>   -              `((h1 "Comparing "
>   +              `((h1 ,page-header ," "
>                        (a (@ (href ,(string-append "/revision/" base-commit)))
>                           (samp ,(string-take base-commit 8) "…"))
>                        " and "
>
> There's a couple of things here. I'd be tempted not to use a variable
> for "Comparing", it's not really the page header, as that's more
> complicated, so I think I'd just use the string in both places.
>
> Second thing, the (if invalid-query? bit when constructing the h1
> element is important. The query parameters being invalid could mean
> anything from the form just hasn't been filled in, to the value isn't
> actually a commit hash, but something else, maybe some HTML/JavaScript
> that is malicious and shouldn't be included in the page. A similar
> approach probably needs taking for the title.
>
>   @@ -419,14 +424,18 @@
>        '(span (@ (class "text-success glyphicon glyphicon-plus pull-left")
>                  (style "font-size: 1.5em; padding-right: 0.4em;"))))
>
>   +  (define page-header "Comparing derivations")
>   +
>      (layout
>   +   #:title
>   +   page-header
>       #:body
>       `(,(header)
>         (div
>          (@ (class "container"))
>          (div
>           (@ (class "row"))
>   -       (h1 ,@(let ((base-commit (assq-ref query-parameters 'base_commit))
>   +       (h1 ,(let ((base-commit (assq-ref query-parameters  'base_commit))
>
> Why's the @ being removed here?
>
>   @@ -435,7 +444,7 @@
>                         " and "
>                         (a (@ (href ,(string-append "/revision/" target-commit)))
>                            (samp ,(string-take target-commit 8) "…")))
>   -                   '("Comparing derivations")))))
>   +                    `,page-header))))
>
> The quote then immediate unquote here isn't necessary, also, I think
> this should stick to being a list containing a string, as the other part
> of the if returns a list.
>
>   diff --git a/guix-data-service/web/dumps/html.scm b/guix-data-service/web/dumps/html.scm
>   index 71e69c8..9645f7c 100644
>   --- a/guix-data-service/web/dumps/html.scm
>   +++ b/guix-data-service/web/dumps/html.scm
>   @@ -21,8 +21,13 @@
>      #:use-module (guix-data-service web view html)
>      #:export (view-dumps))
>
>   +(define page-header "Database dumps")
>   +
>    (define (view-dumps available-dumps)
>   +
>      (layout
>   +   #:title
>   +   page-header
>       #:body
>       `(,(header)
>         (div
>   @@ -31,7 +36,7 @@
>           (@ (class "row"))
>           (div
>            (@ (class "col-sm-12"))
>   -        (h1 "Database dumps")))
>   +        (h1 ,page-header)))
>
> Like the others, I'd probably put page-header inside the view-dumps
> procedure. Same goes for other places where it's outside.
>
>   @@ -267,7 +279,7 @@
>            (@ (class "col-sm-12"))
>            (a (@ (href "/jobs"))
>               (h3 "Jobs"))
>   -        (h1 "Queued jobs ("
>   +        (h1 ,page-header"("
>                ,(length jobs-and-events)
>                ")")))
>          (div
>
> I'd suspect the title here would be "Queued jobs(", I'd also put a space
> between ,page-header the bit after it in the code.
>
>   @@ -329,8 +341,13 @@
>                                 '())))))
>                     jobs-and-events)))))))))
>
>   +
>    (define (view-job job-id query-parameters log)
>   +  (define page-header (string-append "Job " job-id))
>   +
>      (layout
>   +   #:title
>   +   page-header
>       #:body
>       `(,(header)
>         (div
>
> Most of the procedures are separated by one line, and I wouldn't change
> that here.
>
>   @@ -24,7 +24,11 @@
>      #:export (view-package))
>
>    (define* (view-package name package-version-with-branches)
>   +  (define page-header "Package")
>   +
>      (layout
>   +   #:title
>   +   (string-append page-header " " name)
>       #:body
>       `(,(header)
>         (div
>   @@ -33,7 +37,7 @@
>           (@ (class "row"))
>           (div
>            (@ (class "col-md-12"))
>   -        (h1 "Package: " ,name)))
>   +        (h1 ,page-header ," " ,name)))
>          ,@(map
>             (match-lambda
>               ((('version . version)
>
> I'm not that fussed about the colon, but I'd probably keep it.
>
> I'd try to keep the page-header variable meaningful if you're going to
> use it though. "Package" is named as the page-header, but it's not what
> the title is, or the h1 element. They're both better as they include the
> package name, I'd probably just make the page-header the actual string
> used in both places.
>
>   @@ -65,7 +69,11 @@
>    (define* (view-git-repository git-repository-id
>                                  label url cgit-url-base
>                                  branches-with-most-recent-commits)
>   +  (define page-header (string-append "Repository " (string-drop url 8)))
>   +
>      (layout
>   +   #:title
>   +   page-header
>       #:body
>       `(,(header)
>         (div
>
> This is really nice, it's good that the pages for different repositories
> won't have the same title.
>
>   @@ -197,7 +209,11 @@
>                                 branch-name
>                                 package-name
>                                 versions-by-revision-range)
>   +  (define page-header (string-append branch-name " " package-name))
>   +
>      (layout
>   +   #:title
>   +   page-header
>       #:body
>       `(,(header)
>         (div
>
> I think you might need some more words for this title to make sense
> though, even just having package-name " on " branch-name would probably
> help people work out what's going on.
>
>   @@ -386,6 +402,8 @@
>                    (map first derivations-by-revision-range))))
>
>      (layout
>   +   #:title
>   +   (string-append package-name " Package derivations")
>       #:body
>       `(,(header)
>         (div
>   @@ -636,6 +654,8 @@
>                    (map first outputs-by-revision-range))))
>
>      (layout
>   +   #:title
>   +   (string-append package-name " Package outputs")
>       #:body
>       `(,(header)
>         (div
>   @@ -849,6 +869,8 @@
>                                             valid-systems
>                                             system-test-history)
>      (layout
>   +   #:title
>   +   (string-append system-test-name " History")
>       #:body
>       `(,(header)
>         (div
>
> Given there's something coming first in these titles, I wouldn't
> capitalise the H in History or P in Package, since it's not at the start
> of the title.
>
>   @@ -48,7 +48,12 @@
>    (define* (view-revision-news commit-hash
>                                 query-parameters
>                                 news-entries)
>   +  (define page-header "Revision")
>   +
>      (layout
>   +   #:title
>   +   (string-append "Channel News Entries " page-header " "
>   +    (string-take commit-hash 7))
>       #:body
>       `(,(header)
>         (div
>
>   @@ -48,7 +48,12 @@
>    (define* (view-revision-news commit-hash
>                                 query-parameters
>                                 news-entries)
>   +  (define page-header "Revision")
>   +
>      (layout
>   +   #:title
>   +   (string-append "Channel News Entries " page-header " "
>   +    (string-take commit-hash 7))
>       #:body
>       `(,(header)
>         (div
>
> Same thing here regarding defining a page-header which isn't actually
> used as the header.
>
> I do think it's useful to include both the Channel News Entries bit and
> the revision bit in the title, but I'd probably put a separator (like a
> -) in between them. Same goes for the other changes in this file.
>
>   @@ -2314,7 +2386,11 @@ figure {
>
>    (define (unknown-revision commit-hash job git-repositories-and-branches
>                              jobs-and-events)
>   +  (define page-header "Uknown revision")
>   +
>      (layout
>   +   #:title
>   +   page-header
>       #:body
>       `(,(header)
>         (div
>
> A letter was lost here.

[-- Attachment #2: 0001-Set-a-more-informative-page-title-for-any-page.patch --]
[-- Type: text/x-patch, Size: 20590 bytes --]

From a3c708627df0be4f3b530d2fcd074bb943705aa4 Mon Sep 17 00:00:00 2001
From: Canan Talayhan <canan.t.talayhan@gmail.com>
Date: Sun, 18 Apr 2021 23:27:07 +0300
Subject: [PATCH] Set a more informative page title for any page where the
 title is "Guix Data Service"

---
 guix-data-service/web/build-server/html.scm | 24 ++++++--
 guix-data-service/web/build/html.scm        |  6 +-
 guix-data-service/web/compare/html.scm      | 25 ++++++--
 guix-data-service/web/dumps/html.scm        |  6 +-
 guix-data-service/web/jobs/html.scm         | 27 +++++++--
 guix-data-service/web/nar/html.scm          |  5 +-
 guix-data-service/web/package/html.scm      |  3 +
 guix-data-service/web/repository/html.scm   | 30 +++++++++-
 guix-data-service/web/revision/html.scm     | 65 +++++++++++++++++++--
 guix-data-service/web/view/html.scm         | 12 ++--
 10 files changed, 176 insertions(+), 27 deletions(-)

diff --git a/guix-data-service/web/build-server/html.scm b/guix-data-service/web/build-server/html.scm
index f16a570..541a960 100644
--- a/guix-data-service/web/build-server/html.scm
+++ b/guix-data-service/web/build-server/html.scm
@@ -27,7 +27,11 @@
 (define (view-build query-parameters
                     build
                     required-failed-builds)
+  (define page-header "Build")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -36,7 +40,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Build")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        ,@(match build
@@ -98,7 +102,11 @@
             '())))))
 
 (define (view-build-servers build-servers)
+  (define page-header "Build servers")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -107,7 +115,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build servers")
+        (h2 ,page-header)
         ,@(map
            (match-lambda
              ((id url lookup-all-derivations? lookup-builds?)
@@ -127,7 +135,11 @@
            build-servers)))))))
 
 (define (view-build-server build-server)
+  (define page-header "Build server")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -136,7 +148,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build server")
+        (h2 ,page-header)
         ,(match build-server
            ((url lookup-all-derivations?)
             `(dl
@@ -150,7 +162,11 @@
                        "No")))))))))))
 
 (define (view-signing-key sexp)
+  (define page-header "Signing key")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -159,5 +175,5 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Signing key")
+        (h2 ,page-header)
         ,(sexp-div sexp)))))))
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/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..42ef0ef 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -97,6 +97,8 @@
       (query-parameters->string query-parameters)))
 
   (layout
+   #:title
+   "Compare"
    #:body
    `(,(header)
      (div
@@ -420,13 +422,15 @@
               (style "font-size: 1.5em; padding-right: 0.4em;"))))
 
   (layout
+   #:title
+   "Comparing derivations"
    #:body
    `(,(header)
      (div
       (@ (class "container"))
       (div
        (@ (class "row"))
-       (h1 ,@(let ((base-commit (assq-ref query-parameters 'base_commit))
+       (h1 ,(let ((base-commit (assq-ref query-parameters 'base_commit))
                    (target-commit (assq-ref query-parameters 'target_commit)))
                (if (every string? (list base-commit target-commit))
                    `("Comparing "
@@ -435,7 +439,7 @@
                      " and "
                      (a (@ (href ,(string-append "/revision/" target-commit)))
                         (samp ,(string-take target-commit 8) "…")))
-                   '("Comparing derivations")))))
+                    '("Comparing derivations")))))
       (div
        (@ (class "row"))
        (div
@@ -685,7 +689,11 @@
   (define fields
     (assq-ref query-parameters 'field))
 
+  (define page-header "Package derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -835,7 +843,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Package derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? derivation-changes)
           '(p "No derivation changes")
@@ -950,7 +958,12 @@ enough builds to determine a change")))
     (string-append "?base_commit=" base-commit
                    "&target_commit=" target-commit))
 
+  (define page-header (string-append "Comparing "
+    (string-take base-commit 8) " and " (string-take target-commit 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1042,7 +1055,11 @@ enough builds to determine a change")))
                                           #:optional
                                           base-revision-details
                                           target-revision-details)
+  (define page-header "System test derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1141,7 +1158,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "System test derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? changes)
           '(p "No system test derivation changes")
diff --git a/guix-data-service/web/dumps/html.scm b/guix-data-service/web/dumps/html.scm
index 71e69c8..d6d77f9 100644
--- a/guix-data-service/web/dumps/html.scm
+++ b/guix-data-service/web/dumps/html.scm
@@ -22,7 +22,11 @@
   #:export (view-dumps))
 
 (define (view-dumps available-dumps)
+  (define page-header "Database 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..373387d 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 (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Recent events")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -256,7 +264,12 @@
              recent-events)))))))))
 
 (define (view-job-queue jobs-and-events)
+  (define page-header (string-append "Queued jobs ("
+    (number->string (length jobs-and-events)) ")"))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -267,9 +280,7 @@
         (@ (class "col-sm-12"))
         (a (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Queued jobs ("
-            ,(length jobs-and-events)
-            ")")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -330,7 +341,11 @@
                  jobs-and-events)))))))))
 
 (define (view-job job-id query-parameters log)
+  (define page-header (string-append "Job " job-id))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -339,7 +354,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Job " ,job-id)))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
diff --git a/guix-data-service/web/nar/html.scm b/guix-data-service/web/nar/html.scm
index 596d16b..063b091 100644
--- a/guix-data-service/web/nar/html.scm
+++ b/guix-data-service/web/nar/html.scm
@@ -22,7 +22,10 @@
   #:export (view-substitutes))
 
 (define (view-substitutes narinfo-signing-public-key)
+  (define page-header "Substitutes")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -31,7 +34,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Substitutes")
+        (h1 ,page-header)
         ,@(if (canonical-sexp? narinfo-signing-public-key)
               `((h3 "Public key")
                 (pre
diff --git a/guix-data-service/web/package/html.scm b/guix-data-service/web/package/html.scm
index 0d9b078..85b33e9 100644
--- a/guix-data-service/web/package/html.scm
+++ b/guix-data-service/web/package/html.scm
@@ -24,7 +24,10 @@
   #:export (view-package))
 
 (define* (view-package name package-version-with-branches)
+
   (layout
+   #:title
+   (string-append "Package: " name)
    #:body
    `(,(header)
      (div
diff --git a/guix-data-service/web/repository/html.scm b/guix-data-service/web/repository/html.scm
index 88f2632..4bb50db 100644
--- a/guix-data-service/web/repository/html.scm
+++ b/guix-data-service/web/repository/html.scm
@@ -32,7 +32,11 @@
             view-no-latest-revision))
 
 (define* (view-git-repositories git-repositories)
+   (define page-header "Git repositories")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -41,7 +45,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-md-12"))
-        (h1 "Git repositories")))
+        (h1 ,page-header)))
       ,@(map
          (match-lambda
            ((id label url cgit-base-url)
@@ -65,7 +69,11 @@
 (define* (view-git-repository git-repository-id
                               label url cgit-url-base
                               branches-with-most-recent-commits)
+  (define page-header (string-append "Repository " (string-drop url 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -86,7 +94,11 @@
 
 (define (view-branch git-repository-id
                      branch-name query-parameters branch-commits)
+  (define page-header (string-append branch-name " branch"))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -197,7 +209,11 @@
                              branch-name
                              package-name
                              versions-by-revision-range)
+  (define page-header (string-append package-name " on " branch-name))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -386,6 +402,8 @@
                 (map first derivations-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " package derivations")
    #:body
    `(,(header)
      (div
@@ -636,6 +654,8 @@
                 (map first outputs-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " package outputs")
    #:body
    `(,(header)
      (div
@@ -849,6 +869,8 @@
                                          valid-systems
                                          system-test-history)
   (layout
+   #:title
+   (string-append system-test-name " history")
    #:body
    `(,(header)
      (div
@@ -1016,12 +1038,16 @@
                 '(#f))))))))))))
 
 (define (view-no-latest-revision branch-name)
+   (define page-header "No latest revision")
+
   (layout
+   #:title
+   (string-append page-header " for " branch-name)
    #:body
    `(,(header)
      (div
       (@ (class "container"))
-      (h1 "No latest revision")
+      (h1 ,page-header)
       (p "No latest revision for "
          (strong (samp ,branch-name))
          " branch")))))
diff --git a/guix-data-service/web/revision/html.scm b/guix-data-service/web/revision/html.scm
index 25b79f4..514129b 100644
--- a/guix-data-service/web/revision/html.scm
+++ b/guix-data-service/web/revision/html.scm
@@ -48,7 +48,11 @@
 (define* (view-revision-news commit-hash
                              query-parameters
                              news-entries)
+
   (layout
+   #:title
+   (string-append "Channel News Entries - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -107,7 +111,11 @@
                                 #:key path-base
                                 header-text
                                 header-link)
+
   (layout
+   #:title
+   (string-append "Package: " name " - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -135,7 +143,7 @@
                                          branch-name))))
                   branches)))
           git-repositories-and-branches)
-        (h1 "Package " ,name)))
+        (h1 "Package: " ,name)))
       (div
        (@ (class "row"))
        (div
@@ -169,7 +177,11 @@
                                             #:key header-text
                                             header-link
                                             version-history-link)
+
   (layout
+   #:title
+   (string-append "Package: " name " @ " version " - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -224,7 +236,7 @@
                       (role "button"))
                    "Version history"))
               '())
-        (h1 "Package " ,name " @ " ,version)))
+        (h1 "Package: " ,name " @ " ,version)))
       (div
        (@ (class "row"))
        (div
@@ -471,7 +483,10 @@
                         lint-warning-counts
                         #:key (path-base "/revision/")
                         header-text)
+
   (layout
+   #:title
+   (string-append "Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -548,6 +563,9 @@
        "Home page" "Location" "Licenses")))
 
   (layout
+   #:title
+   (string-append  "Packages - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -756,6 +774,9 @@
      package-description-counts))
 
   (layout
+   #:title
+   (string-append "Packages translation availability - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -835,7 +856,10 @@
                                      query-parameters
                                      #:key (path-base "/revision/")
                                      header-text header-link)
+
   (layout
+   #:title
+   (string-append  "System tests - Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -936,7 +960,11 @@
                                           channel-instances
                                           #:key (path-base "/revision/")
                                           header-text header-link)
+
   (layout
+   #:title
+   (string-append "Channel instances - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1217,6 +1245,9 @@ figure {
                 colours))))))
 
   (layout
+   #:title
+   (string-append "Package substitute availability - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style ,chart-css)
@@ -1254,7 +1285,11 @@ figure {
                                                 #:key (path-base "/revision/")
                                                 header-text
                                                 header-link)
+
   (layout
+   #:title
+   (string-append  "Package reproducibility - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style "
@@ -1522,6 +1557,9 @@ figure {
     (assq-ref query-parameters 'field))
 
   (layout
+   #:title
+   (string-append  "Package derivations - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1703,6 +1741,9 @@ figure {
       ))
 
   (layout
+   #:title
+   (string-append  "Fixed output package derivations - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1843,6 +1884,9 @@ figure {
          build-server-urls))
 
   (layout
+   #:title
+   (string-append "Package derivation outputs - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2021,7 +2065,10 @@ figure {
                               build-server-options
                               stats
                               builds)
+
   (layout
+   #:title
+   (string-append  "Builds - Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2159,6 +2206,9 @@ figure {
      '("Linter" "Message" "Location")))
 
   (layout
+   #:title
+   (string-append "Lint warnings - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2314,7 +2364,11 @@ figure {
 
 (define (unknown-revision commit-hash job git-repositories-and-branches
                           jobs-and-events)
+  (define page-header "Unknown revision")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2347,13 +2401,16 @@ figure {
                 (strong (@ (class "text-center")
                            (style "font-size: 2em; display: block;"))
                         "Unknown"))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
 
 (define (unprocessed-revision commit-hash job git-repositories-and-branches
                               jobs-and-events)
+  (define page-header "Unknown revision")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2375,6 +2432,6 @@ figure {
                        git-repositories-and-branches
                        commit-hash))
                 ,@(view-revision/jobs-and-events jobs-and-events))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
diff --git a/guix-data-service/web/view/html.scm b/guix-data-service/web/view/html.scm
index 8063e17..33535a9 100644
--- a/guix-data-service/web/view/html.scm
+++ b/guix-data-service/web/view/html.scm
@@ -65,13 +65,15 @@
 (define* (layout #:key
                  (head '())
                  (body '())
-                 (title "Guix Data Service")
+                 title
                  description)
   `((doctype "html")
     (html
      (@ (lang "en"))
      (head
-      (title ,title)
+      (title ,(if title
+               (string-append title " — Guix Data Service")
+               "Guix Data Service"))
       (meta (@ (http-equiv "Content-Type")
                (content "text/html; charset=UTF-8")))
       (meta (@ (name "viewport")
@@ -286,8 +288,7 @@
 (define (index git-repositories-and-revisions)
   (layout
    #:description
-   "The Guix Data Service processes, stores and provides data about Guix over
-time."
+   "The Guix Data Service processes, stores and provides data about Guix over time."
    #:body
    `(,(header)
      (div
@@ -334,7 +335,10 @@ time."
          git-repositories-and-revisions)))))
 
 (define (view-statistics guix-revisions-count derivations-count)
+  (define page-header "Statistics")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
-- 
2.17.1

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 5775 bytes --]


Canan Talayhan <canan.t.talayhan@gmail.com> writes:

> Thanks for your quick response.
>
>>Why's the @ being removed here?
> It interprets like an HTML code when I use the page-header like
> `,page-header, so I removed it. According to your comment, I reverted
> to the original version.
>
> " 'GET repository..." which includes package/package-name in the URL
> has not the best titles since I couldn't test them because of the
> error that I've mentioned.
> I'm open to suggestions.
>
> Could you please re-review the patch that contains all the
> modifications you've mentioned in the previous message?

I've had another look, see my comments below.

> On Sun, Apr 18, 2021 at 8:53 PM Christopher Baines <mail@cbaines.net> wrote:
>>
>>
>> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
>>
>> > I've updated the patch that contains all the suggestions. I think the patch
>> > is ready to merge.
>> >
>> > One thing that I would like to ask you about the package and package-name
>> > in web/repository/controller.scm.
>> >
>> > When I test the URL below I'm getting this error. (
>> > https://pastebin.ubuntu.com/p/HdKShmKqH7/)
>> >
>> >    - ('GET "repository" repository-id "branch" branch-name "package"
>> >    package-name) ->
>> >    http://localhost:8765/repository/1/branch/master/package/emacs
>> >
>> > What do you think? BTW it's accessible on the official server.
>> >
>> >    - https://data.guix.gnu.org/repository/1/branch/master/package/emacs/
>>
>> Hmm, this could possibly be due to an issue with the small dump of the
>> database.
>>
>> > Could you please review the patch attached?
>> > I'm very excited to make my first FOSS contribution. :)
>>
>> I've had a look though, and I have some more comments:
>>
>>   diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
>>   index 5b5fe0a..170fb12 100644
>>   --- a/guix-data-service/web/compare/html.scm
>>   +++ b/guix-data-service/web/compare/html.scm
>>   @@ -96,7 +96,12 @@
>>        (unless invalid-query?
>>          (query-parameters->string query-parameters)))
>>
>>   +  (define page-header "Comparing")
>>   +
>>      (layout
>>   +   #:title
>>   +   (string-append page-header " " (string-take base-commit 8) " and "
>>   +    (string-take target-commit 8))
>>       #:body
>>       `(,(header)
>>         (div
>>   @@ -107,7 +112,7 @@
>>            (@ (class "col-sm-7"))
>>            ,@(if invalid-query?
>>                  `((h1 "Compare"))
>>   -              `((h1 "Comparing "
>>   +              `((h1 ,page-header ," "
>>                        (a (@ (href ,(string-append "/revision/" base-commit)))
>>                           (samp ,(string-take base-commit 8) "…"))
>>                        " and "
>>
>> There's a couple of things here. I'd be tempted not to use a variable
>> for "Comparing", it's not really the page header, as that's more
>> complicated, so I think I'd just use the string in both places.
>>
>> Second thing, the (if invalid-query? bit when constructing the h1
>> element is important. The query parameters being invalid could mean
>> anything from the form just hasn't been filled in, to the value isn't
>> actually a commit hash, but something else, maybe some HTML/JavaScript
>> that is malicious and shouldn't be included in the page. A similar
>> approach probably needs taking for the title.

This stuff above still looks the same to me, although part of my
analysis was wrong, as the type of an invalid parameter is a record, so
the page just breaks if the parameters are invalid (which I guses is
better than what I was describing).

Anyway, I think this still needs fixing.

>>   @@ -419,14 +424,18 @@
>>        '(span (@ (class "text-success glyphicon glyphicon-plus pull-left")
>>                  (style "font-size: 1.5em; padding-right: 0.4em;"))))
>>
>>   +  (define page-header "Comparing derivations")
>>   +
>>      (layout
>>   +   #:title
>>   +   page-header
>>       #:body
>>       `(,(header)
>>         (div
>>          (@ (class "container"))
>>          (div
>>           (@ (class "row"))
>>   -       (h1 ,@(let ((base-commit (assq-ref query-parameters 'base_commit))
>>   +       (h1 ,(let ((base-commit (assq-ref query-parameters  'base_commit))
>>
>> Why's the @ being removed here?

This is still being removed.

  @@ -435,7 +439,7 @@
                        " and "
                        (a (@ (href ,(string-append "/revision/" target-commit)))
                           (samp ,(string-take target-commit 8) "…")))
  -                   '("Comparing derivations")))))
  +                    '("Comparing derivations")))))
         (div
          (@ (class "row"))
          (div

Watch out for unwanted indentation changes you're making, I think the
previous indentation was better.

  @@ -256,7 +264,12 @@
                recent-events)))))))))

   (define (view-job-queue jobs-and-events)
  +  (define page-header (string-append "Queued jobs ("
  +    (number->string (length jobs-and-events)) ")"))
  +
     (layout
  +   #:title
  +   page-header
      #:body
      `(,(header)
        (div

For the page-header here, I'd indent the code like so:

  (define page-header
    (string-append "Queued jobs ("
                   (number->string (length jobs-and-events))
                   ")"))

The main change being moving the code off the (define ... line. If it's
a one line thing that can fit, I think it's fine to have as one line,
but otherwise, I think this makes it easier to read.

The other thing to fix before merging is the commit message. The current
one is a bit odd, and the first line is too long.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

[-- Attachment #1.1: Type: text/plain, Size: 7845 bytes --]

I've missed it unintentionally. I've not touched the "@" sign this time. :)

>>  + (define page-header "Comparing")
 >>  +
 >>   (layout
 >>  +  #:title
 >>  +  (string-append page-header " " (string-take base-commit 8) " and "
 >>  +  (string-take target-commit 8))
 >>    #:body
 >>    `(,(header)
 >>     (div
 >>  @@ -107,7 +112,7 @@
 >>      (@ (class "col-sm-7"))
 >>      ,@(if invalid-query?
 >>         `((h1 "Compare"))
 >>  -       `((h1 "Comparing "
 >>  +       `((h1 ,page-header ," "
 >>            (a (@ (href ,(string-append "/revision/" base-commit)))
 >>              (samp ,(string-take base-commit 8) "…"))
 >>            " and "

I think I misunderstood that comment.
>> There's a couple of things here. I'd be tempted not to use a variable
>> for "Comparing", it's not really the page header, as that's more
>> complicated, so I think I'd just use the string in both places.

Now, I've fixed it this way. I hope this version is good.


















*(layout   #:title   (string-append "Comparing " (string-take base-commit
8) " and "    (string-take target-commit 8))   #:body   `(,(header)
 (div      (@ (class "container"))      (div       (@ (class "row"))
 (div        (@ (class "col-sm-7"))        ,@(if invalid-query?
  `((h1 "Compare"))              `((h1 "Comparing "                    (a
(@ (href ,(string-append "/revision/" base-commit)))
 (samp ,(string-take base-commit 8) "…"))                    " and "*

I'm using VS Code, and sometimes it adds odd spaces to the code. Maybe I
need to switch to another code editor.

Thanks for all,
Canan Talayhan


On Mon, Apr 19, 2021 at 10:16 PM Christopher Baines <mail@cbaines.net>
wrote:

>
> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
>
> > Thanks for your quick response.
> >
> >>Why's the @ being removed here?
> > It interprets like an HTML code when I use the page-header like
> > `,page-header, so I removed it. According to your comment, I reverted
> > to the original version.
> >
> > " 'GET repository..." which includes package/package-name in the URL
> > has not the best titles since I couldn't test them because of the
> > error that I've mentioned.
> > I'm open to suggestions.
> >
> > Could you please re-review the patch that contains all the
> > modifications you've mentioned in the previous message?
>
> I've had another look, see my comments below.
>
> > On Sun, Apr 18, 2021 at 8:53 PM Christopher Baines <mail@cbaines.net>
> wrote:
> >>
> >>
> >> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
> >>
> >> > I've updated the patch that contains all the suggestions. I think the
> patch
> >> > is ready to merge.
> >> >
> >> > One thing that I would like to ask you about the package and
> package-name
> >> > in web/repository/controller.scm.
> >> >
> >> > When I test the URL below I'm getting this error. (
> >> > https://pastebin.ubuntu.com/p/HdKShmKqH7/)
> >> >
> >> >    - ('GET "repository" repository-id "branch" branch-name "package"
> >> >    package-name) ->
> >> >    http://localhost:8765/repository/1/branch/master/package/emacs
> >> >
> >> > What do you think? BTW it's accessible on the official server.
> >> >
> >> >    -
> https://data.guix.gnu.org/repository/1/branch/master/package/emacs/
> >>
> >> Hmm, this could possibly be due to an issue with the small dump of the
> >> database.
> >>
> >> > Could you please review the patch attached?
> >> > I'm very excited to make my first FOSS contribution. :)
> >>
> >> I've had a look though, and I have some more comments:
> >>
> >>   diff --git a/guix-data-service/web/compare/html.scm
> b/guix-data-service/web/compare/html.scm
> >>   index 5b5fe0a..170fb12 100644
> >>   --- a/guix-data-service/web/compare/html.scm
> >>   +++ b/guix-data-service/web/compare/html.scm
> >>   @@ -96,7 +96,12 @@
> >>        (unless invalid-query?
> >>          (query-parameters->string query-parameters)))
> >>
> >>   +  (define page-header "Comparing")
> >>   +
> >>      (layout
> >>   +   #:title
> >>   +   (string-append page-header " " (string-take base-commit 8) " and "
> >>   +    (string-take target-commit 8))
> >>       #:body
> >>       `(,(header)
> >>         (div
> >>   @@ -107,7 +112,7 @@
> >>            (@ (class "col-sm-7"))
> >>            ,@(if invalid-query?
> >>                  `((h1 "Compare"))
> >>   -              `((h1 "Comparing "
> >>   +              `((h1 ,page-header ," "
> >>                        (a (@ (href ,(string-append "/revision/"
> base-commit)))
> >>                           (samp ,(string-take base-commit 8) "…"))
> >>                        " and "
> >>
> >> There's a couple of things here. I'd be tempted not to use a variable
> >> for "Comparing", it's not really the page header, as that's more
> >> complicated, so I think I'd just use the string in both places.
> >>
> >> Second thing, the (if invalid-query? bit when constructing the h1
> >> element is important. The query parameters being invalid could mean
> >> anything from the form just hasn't been filled in, to the value isn't
> >> actually a commit hash, but something else, maybe some HTML/JavaScript
> >> that is malicious and shouldn't be included in the page. A similar
> >> approach probably needs taking for the title.
>
> This stuff above still looks the same to me, although part of my
> analysis was wrong, as the type of an invalid parameter is a record, so
> the page just breaks if the parameters are invalid (which I guses is
> better than what I was describing).
>
> Anyway, I think this still needs fixing.
>
> >>   @@ -419,14 +424,18 @@
> >>        '(span (@ (class "text-success glyphicon glyphicon-plus
> pull-left")
> >>                  (style "font-size: 1.5em; padding-right: 0.4em;"))))
> >>
> >>   +  (define page-header "Comparing derivations")
> >>   +
> >>      (layout
> >>   +   #:title
> >>   +   page-header
> >>       #:body
> >>       `(,(header)
> >>         (div
> >>          (@ (class "container"))
> >>          (div
> >>           (@ (class "row"))
> >>   -       (h1 ,@(let ((base-commit (assq-ref query-parameters
> 'base_commit))
> >>   +       (h1 ,(let ((base-commit (assq-ref query-parameters
> 'base_commit))
> >>
> >> Why's the @ being removed here?
>
> This is still being removed.
>
>   @@ -435,7 +439,7 @@
>                         " and "
>                         (a (@ (href ,(string-append "/revision/"
> target-commit)))
>                            (samp ,(string-take target-commit 8) "…")))
>   -                   '("Comparing derivations")))))
>   +                    '("Comparing derivations")))))
>          (div
>           (@ (class "row"))
>           (div
>
> Watch out for unwanted indentation changes you're making, I think the
> previous indentation was better.
>
>   @@ -256,7 +264,12 @@
>                 recent-events)))))))))
>
>    (define (view-job-queue jobs-and-events)
>   +  (define page-header (string-append "Queued jobs ("
>   +    (number->string (length jobs-and-events)) ")"))
>   +
>      (layout
>   +   #:title
>   +   page-header
>       #:body
>       `(,(header)
>         (div
>
> For the page-header here, I'd indent the code like so:
>
>   (define page-header
>     (string-append "Queued jobs ("
>                    (number->string (length jobs-and-events))
>                    ")"))
>
> The main change being moving the code off the (define ... line. If it's
> a one line thing that can fit, I think it's fine to have as one line,
> but otherwise, I think this makes it easier to read.
>
> The other thing to fix before merging is the commit message. The current
> one is a bit odd, and the first line is too long.
>

[-- Attachment #1.2: Type: text/html, Size: 11018 bytes --]

[-- Attachment #2: 0001-Set-a-more-informative-page-titles.patch --]
[-- Type: text/x-patch, Size: 19946 bytes --]

From 2690fc5924ca7617ef51aeb63cd74318bc63a3fd Mon Sep 17 00:00:00 2001
From: Canan Talayhan <canan.t.talayhan@gmail.com>
Date: Wed, 21 Apr 2021 17:18:13 +0300
Subject: [PATCH] Set a more informative page titles

Any page where the title is "Guix Data Service"
---
 guix-data-service/web/build-server/html.scm | 24 ++++++--
 guix-data-service/web/build/html.scm        |  6 +-
 guix-data-service/web/compare/html.scm      | 22 ++++++-
 guix-data-service/web/dumps/html.scm        |  6 +-
 guix-data-service/web/jobs/html.scm         | 29 +++++++--
 guix-data-service/web/nar/html.scm          |  5 +-
 guix-data-service/web/package/html.scm      |  3 +
 guix-data-service/web/repository/html.scm   | 30 +++++++++-
 guix-data-service/web/revision/html.scm     | 65 +++++++++++++++++++--
 guix-data-service/web/view/html.scm         | 12 ++--
 10 files changed, 177 insertions(+), 25 deletions(-)

diff --git a/guix-data-service/web/build-server/html.scm b/guix-data-service/web/build-server/html.scm
index f16a570..541a960 100644
--- a/guix-data-service/web/build-server/html.scm
+++ b/guix-data-service/web/build-server/html.scm
@@ -27,7 +27,11 @@
 (define (view-build query-parameters
                     build
                     required-failed-builds)
+  (define page-header "Build")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -36,7 +40,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Build")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        ,@(match build
@@ -98,7 +102,11 @@
             '())))))
 
 (define (view-build-servers build-servers)
+  (define page-header "Build servers")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -107,7 +115,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build servers")
+        (h2 ,page-header)
         ,@(map
            (match-lambda
              ((id url lookup-all-derivations? lookup-builds?)
@@ -127,7 +135,11 @@
            build-servers)))))))
 
 (define (view-build-server build-server)
+  (define page-header "Build server")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -136,7 +148,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build server")
+        (h2 ,page-header)
         ,(match build-server
            ((url lookup-all-derivations?)
             `(dl
@@ -150,7 +162,11 @@
                        "No")))))))))))
 
 (define (view-signing-key sexp)
+  (define page-header "Signing key")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -159,5 +175,5 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Signing key")
+        (h2 ,page-header)
         ,(sexp-div sexp)))))))
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/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..b5e2b57 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -97,6 +97,9 @@
       (query-parameters->string query-parameters)))
 
   (layout
+   #:title
+   (string-append "Comparing " (string-take base-commit 8) " and "
+    (string-take target-commit 8))
    #:body
    `(,(header)
      (div
@@ -420,6 +423,8 @@
               (style "font-size: 1.5em; padding-right: 0.4em;"))))
 
   (layout
+   #:title
+   "Comparing derivations"
    #:body
    `(,(header)
      (div
@@ -685,7 +690,11 @@
   (define fields
     (assq-ref query-parameters 'field))
 
+  (define page-header "Package derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -835,7 +844,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Package derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? derivation-changes)
           '(p "No derivation changes")
@@ -950,7 +959,12 @@ enough builds to determine a change")))
     (string-append "?base_commit=" base-commit
                    "&target_commit=" target-commit))
 
+  (define page-header (string-append "Comparing "
+    (string-take base-commit 8) " and " (string-take target-commit 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1042,7 +1056,11 @@ enough builds to determine a change")))
                                           #:optional
                                           base-revision-details
                                           target-revision-details)
+  (define page-header "System test derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1141,7 +1159,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "System test derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? changes)
           '(p "No system test derivation changes")
diff --git a/guix-data-service/web/dumps/html.scm b/guix-data-service/web/dumps/html.scm
index 71e69c8..d6d77f9 100644
--- a/guix-data-service/web/dumps/html.scm
+++ b/guix-data-service/web/dumps/html.scm
@@ -22,7 +22,11 @@
   #:export (view-dumps))
 
 (define (view-dumps available-dumps)
+  (define page-header "Database 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..5c5881c 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 (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Recent events")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -256,7 +264,14 @@
              recent-events)))))))))
 
 (define (view-job-queue jobs-and-events)
+  (define page-header
+    (string-append "Queued jobs ("
+              (number->string (length jobs-and-events))
+              ")"))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -267,9 +282,7 @@
         (@ (class "col-sm-12"))
         (a (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Queued jobs ("
-            ,(length jobs-and-events)
-            ")")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -330,7 +343,11 @@
                  jobs-and-events)))))))))
 
 (define (view-job job-id query-parameters log)
+  (define page-header (string-append "Job " job-id))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -339,7 +356,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Job " ,job-id)))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
diff --git a/guix-data-service/web/nar/html.scm b/guix-data-service/web/nar/html.scm
index 596d16b..063b091 100644
--- a/guix-data-service/web/nar/html.scm
+++ b/guix-data-service/web/nar/html.scm
@@ -22,7 +22,10 @@
   #:export (view-substitutes))
 
 (define (view-substitutes narinfo-signing-public-key)
+  (define page-header "Substitutes")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -31,7 +34,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Substitutes")
+        (h1 ,page-header)
         ,@(if (canonical-sexp? narinfo-signing-public-key)
               `((h3 "Public key")
                 (pre
diff --git a/guix-data-service/web/package/html.scm b/guix-data-service/web/package/html.scm
index 0d9b078..85b33e9 100644
--- a/guix-data-service/web/package/html.scm
+++ b/guix-data-service/web/package/html.scm
@@ -24,7 +24,10 @@
   #:export (view-package))
 
 (define* (view-package name package-version-with-branches)
+
   (layout
+   #:title
+   (string-append "Package: " name)
    #:body
    `(,(header)
      (div
diff --git a/guix-data-service/web/repository/html.scm b/guix-data-service/web/repository/html.scm
index 88f2632..4bb50db 100644
--- a/guix-data-service/web/repository/html.scm
+++ b/guix-data-service/web/repository/html.scm
@@ -32,7 +32,11 @@
             view-no-latest-revision))
 
 (define* (view-git-repositories git-repositories)
+   (define page-header "Git repositories")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -41,7 +45,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-md-12"))
-        (h1 "Git repositories")))
+        (h1 ,page-header)))
       ,@(map
          (match-lambda
            ((id label url cgit-base-url)
@@ -65,7 +69,11 @@
 (define* (view-git-repository git-repository-id
                               label url cgit-url-base
                               branches-with-most-recent-commits)
+  (define page-header (string-append "Repository " (string-drop url 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -86,7 +94,11 @@
 
 (define (view-branch git-repository-id
                      branch-name query-parameters branch-commits)
+  (define page-header (string-append branch-name " branch"))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -197,7 +209,11 @@
                              branch-name
                              package-name
                              versions-by-revision-range)
+  (define page-header (string-append package-name " on " branch-name))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -386,6 +402,8 @@
                 (map first derivations-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " package derivations")
    #:body
    `(,(header)
      (div
@@ -636,6 +654,8 @@
                 (map first outputs-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " package outputs")
    #:body
    `(,(header)
      (div
@@ -849,6 +869,8 @@
                                          valid-systems
                                          system-test-history)
   (layout
+   #:title
+   (string-append system-test-name " history")
    #:body
    `(,(header)
      (div
@@ -1016,12 +1038,16 @@
                 '(#f))))))))))))
 
 (define (view-no-latest-revision branch-name)
+   (define page-header "No latest revision")
+
   (layout
+   #:title
+   (string-append page-header " for " branch-name)
    #:body
    `(,(header)
      (div
       (@ (class "container"))
-      (h1 "No latest revision")
+      (h1 ,page-header)
       (p "No latest revision for "
          (strong (samp ,branch-name))
          " branch")))))
diff --git a/guix-data-service/web/revision/html.scm b/guix-data-service/web/revision/html.scm
index 25b79f4..514129b 100644
--- a/guix-data-service/web/revision/html.scm
+++ b/guix-data-service/web/revision/html.scm
@@ -48,7 +48,11 @@
 (define* (view-revision-news commit-hash
                              query-parameters
                              news-entries)
+
   (layout
+   #:title
+   (string-append "Channel News Entries - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -107,7 +111,11 @@
                                 #:key path-base
                                 header-text
                                 header-link)
+
   (layout
+   #:title
+   (string-append "Package: " name " - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -135,7 +143,7 @@
                                          branch-name))))
                   branches)))
           git-repositories-and-branches)
-        (h1 "Package " ,name)))
+        (h1 "Package: " ,name)))
       (div
        (@ (class "row"))
        (div
@@ -169,7 +177,11 @@
                                             #:key header-text
                                             header-link
                                             version-history-link)
+
   (layout
+   #:title
+   (string-append "Package: " name " @ " version " - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -224,7 +236,7 @@
                       (role "button"))
                    "Version history"))
               '())
-        (h1 "Package " ,name " @ " ,version)))
+        (h1 "Package: " ,name " @ " ,version)))
       (div
        (@ (class "row"))
        (div
@@ -471,7 +483,10 @@
                         lint-warning-counts
                         #:key (path-base "/revision/")
                         header-text)
+
   (layout
+   #:title
+   (string-append "Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -548,6 +563,9 @@
        "Home page" "Location" "Licenses")))
 
   (layout
+   #:title
+   (string-append  "Packages - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -756,6 +774,9 @@
      package-description-counts))
 
   (layout
+   #:title
+   (string-append "Packages translation availability - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -835,7 +856,10 @@
                                      query-parameters
                                      #:key (path-base "/revision/")
                                      header-text header-link)
+
   (layout
+   #:title
+   (string-append  "System tests - Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -936,7 +960,11 @@
                                           channel-instances
                                           #:key (path-base "/revision/")
                                           header-text header-link)
+
   (layout
+   #:title
+   (string-append "Channel instances - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1217,6 +1245,9 @@ figure {
                 colours))))))
 
   (layout
+   #:title
+   (string-append "Package substitute availability - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style ,chart-css)
@@ -1254,7 +1285,11 @@ figure {
                                                 #:key (path-base "/revision/")
                                                 header-text
                                                 header-link)
+
   (layout
+   #:title
+   (string-append  "Package reproducibility - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style "
@@ -1522,6 +1557,9 @@ figure {
     (assq-ref query-parameters 'field))
 
   (layout
+   #:title
+   (string-append  "Package derivations - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1703,6 +1741,9 @@ figure {
       ))
 
   (layout
+   #:title
+   (string-append  "Fixed output package derivations - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1843,6 +1884,9 @@ figure {
          build-server-urls))
 
   (layout
+   #:title
+   (string-append "Package derivation outputs - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2021,7 +2065,10 @@ figure {
                               build-server-options
                               stats
                               builds)
+
   (layout
+   #:title
+   (string-append  "Builds - Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2159,6 +2206,9 @@ figure {
      '("Linter" "Message" "Location")))
 
   (layout
+   #:title
+   (string-append "Lint warnings - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2314,7 +2364,11 @@ figure {
 
 (define (unknown-revision commit-hash job git-repositories-and-branches
                           jobs-and-events)
+  (define page-header "Unknown revision")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2347,13 +2401,16 @@ figure {
                 (strong (@ (class "text-center")
                            (style "font-size: 2em; display: block;"))
                         "Unknown"))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
 
 (define (unprocessed-revision commit-hash job git-repositories-and-branches
                               jobs-and-events)
+  (define page-header "Unknown revision")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2375,6 +2432,6 @@ figure {
                        git-repositories-and-branches
                        commit-hash))
                 ,@(view-revision/jobs-and-events jobs-and-events))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
diff --git a/guix-data-service/web/view/html.scm b/guix-data-service/web/view/html.scm
index 8063e17..33535a9 100644
--- a/guix-data-service/web/view/html.scm
+++ b/guix-data-service/web/view/html.scm
@@ -65,13 +65,15 @@
 (define* (layout #:key
                  (head '())
                  (body '())
-                 (title "Guix Data Service")
+                 title
                  description)
   `((doctype "html")
     (html
      (@ (lang "en"))
      (head
-      (title ,title)
+      (title ,(if title
+               (string-append title " — Guix Data Service")
+               "Guix Data Service"))
       (meta (@ (http-equiv "Content-Type")
                (content "text/html; charset=UTF-8")))
       (meta (@ (name "viewport")
@@ -286,8 +288,7 @@
 (define (index git-repositories-and-revisions)
   (layout
    #:description
-   "The Guix Data Service processes, stores and provides data about Guix over
-time."
+   "The Guix Data Service processes, stores and provides data about Guix over time."
    #:body
    `(,(header)
      (div
@@ -334,7 +335,10 @@ time."
          git-repositories-and-revisions)))))
 
 (define (view-statistics guix-revisions-count derivations-count)
+  (define page-header "Statistics")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
-- 
2.17.1

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 6037 bytes --]


Canan Talayhan <canan.t.talayhan@gmail.com> writes:

> I've missed it unintentionally. I've not touched the "@" sign this time. :)
>
>>>  + (define page-header "Comparing")
>  >>  +
>  >>   (layout
>  >>  +  #:title
>  >>  +  (string-append page-header " " (string-take base-commit 8) " and "
>  >>  +  (string-take target-commit 8))
>  >>    #:body
>  >>    `(,(header)
>  >>     (div
>  >>  @@ -107,7 +112,7 @@
>  >>      (@ (class "col-sm-7"))
>  >>      ,@(if invalid-query?
>  >>         `((h1 "Compare"))
>  >>  -       `((h1 "Comparing "
>  >>  +       `((h1 ,page-header ," "
>  >>            (a (@ (href ,(string-append "/revision/" base-commit)))
>  >>              (samp ,(string-take base-commit 8) "…"))
>  >>            " and "
>
> I think I misunderstood that comment.
>>> There's a couple of things here. I'd be tempted not to use a variable
>>> for "Comparing", it's not really the page header, as that's more
>>> complicated, so I think I'd just use the string in both places.
>
> Now, I've fixed it this way. I hope this version is good.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> *(layout   #:title   (string-append "Comparing " (string-take base-commit
> 8) " and "    (string-take target-commit 8))   #:body   `(,(header)
>  (div      (@ (class "container"))      (div       (@ (class "row"))
>  (div        (@ (class "col-sm-7"))        ,@(if invalid-query?
>   `((h1 "Compare"))              `((h1 "Comparing "                    (a
> (@ (href ,(string-append "/revision/" base-commit)))
>  (samp ,(string-take base-commit 8) "…"))                    " and "*
>

I think your email came through a bit garbled, anyway, I think there's
still an issue with the title here.

> On Mon, Apr 19, 2021 at 10:16 PM Christopher Baines <mail@cbaines.net>
> wrote:
>
>>
>> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
>>
>> > Thanks for your quick response.
>> >
>> >>Why's the @ being removed here?
>> > It interprets like an HTML code when I use the page-header like
>> > `,page-header, so I removed it. According to your comment, I reverted
>> > to the original version.
>> >
>> > " 'GET repository..." which includes package/package-name in the URL
>> > has not the best titles since I couldn't test them because of the
>> > error that I've mentioned.
>> > I'm open to suggestions.
>> >
>> > Could you please re-review the patch that contains all the
>> > modifications you've mentioned in the previous message?
>>
>> I've had another look, see my comments below.
>>
>> > On Sun, Apr 18, 2021 at 8:53 PM Christopher Baines <mail@cbaines.net>
>> wrote:
>> >>
>> >>
>> >> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
>> >>
>> >> > I've updated the patch that contains all the suggestions. I think the
>> patch
>> >> > is ready to merge.
>> >> >
>> >> > One thing that I would like to ask you about the package and
>> package-name
>> >> > in web/repository/controller.scm.
>> >> >
>> >> > When I test the URL below I'm getting this error. (
>> >> > https://pastebin.ubuntu.com/p/HdKShmKqH7/)
>> >> >
>> >> >    - ('GET "repository" repository-id "branch" branch-name "package"
>> >> >    package-name) ->
>> >> >    http://localhost:8765/repository/1/branch/master/package/emacs
>> >> >
>> >> > What do you think? BTW it's accessible on the official server.
>> >> >
>> >> >    -
>> https://data.guix.gnu.org/repository/1/branch/master/package/emacs/
>> >>
>> >> Hmm, this could possibly be due to an issue with the small dump of the
>> >> database.
>> >>
>> >> > Could you please review the patch attached?
>> >> > I'm very excited to make my first FOSS contribution. :)
>> >>
>> >> I've had a look though, and I have some more comments:
>> >>
>> >>   diff --git a/guix-data-service/web/compare/html.scm
>> b/guix-data-service/web/compare/html.scm
>> >>   index 5b5fe0a..170fb12 100644
>> >>   --- a/guix-data-service/web/compare/html.scm
>> >>   +++ b/guix-data-service/web/compare/html.scm
>> >>   @@ -96,7 +96,12 @@
>> >>        (unless invalid-query?
>> >>          (query-parameters->string query-parameters)))
>> >>
>> >>   +  (define page-header "Comparing")
>> >>   +
>> >>      (layout
>> >>   +   #:title
>> >>   +   (string-append page-header " " (string-take base-commit 8) " and "
>> >>   +    (string-take target-commit 8))
>> >>       #:body
>> >>       `(,(header)
>> >>         (div
>> >>   @@ -107,7 +112,7 @@
>> >>            (@ (class "col-sm-7"))
>> >>            ,@(if invalid-query?
>> >>                  `((h1 "Compare"))
>> >>   -              `((h1 "Comparing "
>> >>   +              `((h1 ,page-header ," "
>> >>                        (a (@ (href ,(string-append "/revision/"
>> base-commit)))
>> >>                           (samp ,(string-take base-commit 8) "…"))
>> >>                        " and "
>> >>
>> >> There's a couple of things here. I'd be tempted not to use a variable
>> >> for "Comparing", it's not really the page header, as that's more
>> >> complicated, so I think I'd just use the string in both places.
>> >>
>> >> Second thing, the (if invalid-query? bit when constructing the h1
>> >> element is important. The query parameters being invalid could mean
>> >> anything from the form just hasn't been filled in, to the value isn't
>> >> actually a commit hash, but something else, maybe some HTML/JavaScript
>> >> that is malicious and shouldn't be included in the page. A similar
>> >> approach probably needs taking for the title.
>>
>> This stuff above still looks the same to me, although part of my
>> analysis was wrong, as the type of an invalid parameter is a record, so
>> the page just breaks if the parameters are invalid (which I guses is
>> better than what I was describing).
>>
>> Anyway, I think this still needs fixing.

These are my relevant comments from before, you should be able to see
the error yourself if you go to /compare locally, does this page work
for you?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

[-- Attachment #1: Type: text/plain, Size: 6771 bytes --]

It seems after testing lots of pages this one escaped me since I only
tested the working case.

Please find the quick fix in the link below.
https://pastebin.ubuntu.com/p/s7tWyPHZ8F/

I'm looking forward to making another contribution. Could you please
review it as soon as possible?

Thanks,
Canan Talayhan


On Thu, Apr 22, 2021 at 10:47 PM Christopher Baines <mail@cbaines.net> wrote:
>
>
> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
>
> > I've missed it unintentionally. I've not touched the "@" sign this time. :)
> >
> >>>  + (define page-header "Comparing")
> >  >>  +
> >  >>   (layout
> >  >>  +  #:title
> >  >>  +  (string-append page-header " " (string-take base-commit 8) " and "
> >  >>  +  (string-take target-commit 8))
> >  >>    #:body
> >  >>    `(,(header)
> >  >>     (div
> >  >>  @@ -107,7 +112,7 @@
> >  >>      (@ (class "col-sm-7"))
> >  >>      ,@(if invalid-query?
> >  >>         `((h1 "Compare"))
> >  >>  -       `((h1 "Comparing "
> >  >>  +       `((h1 ,page-header ," "
> >  >>            (a (@ (href ,(string-append "/revision/" base-commit)))
> >  >>              (samp ,(string-take base-commit 8) "…"))
> >  >>            " and "
> >
> > I think I misunderstood that comment.
> >>> There's a couple of things here. I'd be tempted not to use a variable
> >>> for "Comparing", it's not really the page header, as that's more
> >>> complicated, so I think I'd just use the string in both places.
> >
> > Now, I've fixed it this way. I hope this version is good.
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > *(layout   #:title   (string-append "Comparing " (string-take base-commit
> > 8) " and "    (string-take target-commit 8))   #:body   `(,(header)
> >  (div      (@ (class "container"))      (div       (@ (class "row"))
> >  (div        (@ (class "col-sm-7"))        ,@(if invalid-query?
> >   `((h1 "Compare"))              `((h1 "Comparing "                    (a
> > (@ (href ,(string-append "/revision/" base-commit)))
> >  (samp ,(string-take base-commit 8) "…"))                    " and "*
> >
>
> I think your email came through a bit garbled, anyway, I think there's
> still an issue with the title here.
>
> > On Mon, Apr 19, 2021 at 10:16 PM Christopher Baines <mail@cbaines.net>
> > wrote:
> >
> >>
> >> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
> >>
> >> > Thanks for your quick response.
> >> >
> >> >>Why's the @ being removed here?
> >> > It interprets like an HTML code when I use the page-header like
> >> > `,page-header, so I removed it. According to your comment, I reverted
> >> > to the original version.
> >> >
> >> > " 'GET repository..." which includes package/package-name in the URL
> >> > has not the best titles since I couldn't test them because of the
> >> > error that I've mentioned.
> >> > I'm open to suggestions.
> >> >
> >> > Could you please re-review the patch that contains all the
> >> > modifications you've mentioned in the previous message?
> >>
> >> I've had another look, see my comments below.
> >>
> >> > On Sun, Apr 18, 2021 at 8:53 PM Christopher Baines <mail@cbaines.net>
> >> wrote:
> >> >>
> >> >>
> >> >> Canan Talayhan <canan.t.talayhan@gmail.com> writes:
> >> >>
> >> >> > I've updated the patch that contains all the suggestions. I think the
> >> patch
> >> >> > is ready to merge.
> >> >> >
> >> >> > One thing that I would like to ask you about the package and
> >> package-name
> >> >> > in web/repository/controller.scm.
> >> >> >
> >> >> > When I test the URL below I'm getting this error. (
> >> >> > https://pastebin.ubuntu.com/p/HdKShmKqH7/)
> >> >> >
> >> >> >    - ('GET "repository" repository-id "branch" branch-name "package"
> >> >> >    package-name) ->
> >> >> >    http://localhost:8765/repository/1/branch/master/package/emacs
> >> >> >
> >> >> > What do you think? BTW it's accessible on the official server.
> >> >> >
> >> >> >    -
> >> https://data.guix.gnu.org/repository/1/branch/master/package/emacs/
> >> >>
> >> >> Hmm, this could possibly be due to an issue with the small dump of the
> >> >> database.
> >> >>
> >> >> > Could you please review the patch attached?
> >> >> > I'm very excited to make my first FOSS contribution. :)
> >> >>
> >> >> I've had a look though, and I have some more comments:
> >> >>
> >> >>   diff --git a/guix-data-service/web/compare/html.scm
> >> b/guix-data-service/web/compare/html.scm
> >> >>   index 5b5fe0a..170fb12 100644
> >> >>   --- a/guix-data-service/web/compare/html.scm
> >> >>   +++ b/guix-data-service/web/compare/html.scm
> >> >>   @@ -96,7 +96,12 @@
> >> >>        (unless invalid-query?
> >> >>          (query-parameters->string query-parameters)))
> >> >>
> >> >>   +  (define page-header "Comparing")
> >> >>   +
> >> >>      (layout
> >> >>   +   #:title
> >> >>   +   (string-append page-header " " (string-take base-commit 8) " and "
> >> >>   +    (string-take target-commit 8))
> >> >>       #:body
> >> >>       `(,(header)
> >> >>         (div
> >> >>   @@ -107,7 +112,7 @@
> >> >>            (@ (class "col-sm-7"))
> >> >>            ,@(if invalid-query?
> >> >>                  `((h1 "Compare"))
> >> >>   -              `((h1 "Comparing "
> >> >>   +              `((h1 ,page-header ," "
> >> >>                        (a (@ (href ,(string-append "/revision/"
> >> base-commit)))
> >> >>                           (samp ,(string-take base-commit 8) "…"))
> >> >>                        " and "
> >> >>
> >> >> There's a couple of things here. I'd be tempted not to use a variable
> >> >> for "Comparing", it's not really the page header, as that's more
> >> >> complicated, so I think I'd just use the string in both places.
> >> >>
> >> >> Second thing, the (if invalid-query? bit when constructing the h1
> >> >> element is important. The query parameters being invalid could mean
> >> >> anything from the form just hasn't been filled in, to the value isn't
> >> >> actually a commit hash, but something else, maybe some HTML/JavaScript
> >> >> that is malicious and shouldn't be included in the page. A similar
> >> >> approach probably needs taking for the title.
> >>
> >> This stuff above still looks the same to me, although part of my
> >> analysis was wrong, as the type of an invalid parameter is a record, so
> >> the page just breaks if the parameters are invalid (which I guses is
> >> better than what I was describing).
> >>
> >> Anyway, I think this still needs fixing.
>
> These are my relevant comments from before, you should be able to see
> the error yourself if you go to /compare locally, does this page work
> for you?

[-- Attachment #2: 0001-Set-a-more-informative-page-titles.patch --]
[-- Type: text/x-patch, Size: 19996 bytes --]

From eb21a20037d9765d8339de8ffa4d615366b893a4 Mon Sep 17 00:00:00 2001
From: Canan Talayhan <canan.t.talayhan@gmail.com>
Date: Fri, 23 Apr 2021 11:19:32 +0300
Subject: [PATCH] Set a more informative page titles

Any page where the title is "Guix Data Service"
---
 guix-data-service/web/build-server/html.scm | 24 ++++++--
 guix-data-service/web/build/html.scm        |  6 +-
 guix-data-service/web/compare/html.scm      | 24 +++++++-
 guix-data-service/web/dumps/html.scm        |  6 +-
 guix-data-service/web/jobs/html.scm         | 29 +++++++--
 guix-data-service/web/nar/html.scm          |  5 +-
 guix-data-service/web/package/html.scm      |  3 +
 guix-data-service/web/repository/html.scm   | 30 +++++++++-
 guix-data-service/web/revision/html.scm     | 65 +++++++++++++++++++--
 guix-data-service/web/view/html.scm         | 12 ++--
 10 files changed, 179 insertions(+), 25 deletions(-)

diff --git a/guix-data-service/web/build-server/html.scm b/guix-data-service/web/build-server/html.scm
index f16a570..541a960 100644
--- a/guix-data-service/web/build-server/html.scm
+++ b/guix-data-service/web/build-server/html.scm
@@ -27,7 +27,11 @@
 (define (view-build query-parameters
                     build
                     required-failed-builds)
+  (define page-header "Build")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -36,7 +40,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Build")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        ,@(match build
@@ -98,7 +102,11 @@
             '())))))
 
 (define (view-build-servers build-servers)
+  (define page-header "Build servers")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -107,7 +115,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build servers")
+        (h2 ,page-header)
         ,@(map
            (match-lambda
              ((id url lookup-all-derivations? lookup-builds?)
@@ -127,7 +135,11 @@
            build-servers)))))))
 
 (define (view-build-server build-server)
+  (define page-header "Build server")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -136,7 +148,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Build server")
+        (h2 ,page-header)
         ,(match build-server
            ((url lookup-all-derivations?)
             `(dl
@@ -150,7 +162,11 @@
                        "No")))))))))))
 
 (define (view-signing-key sexp)
+  (define page-header "Signing key")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -159,5 +175,5 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h2 "Signing key")
+        (h2 ,page-header)
         ,(sexp-div sexp)))))))
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/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..62b0991 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -97,6 +97,11 @@
       (query-parameters->string query-parameters)))
 
   (layout
+   #:title
+   (if invalid-query?
+      "Compare"
+      (string-append "Comparing " (string-take base-commit 8) " and "
+        (string-take target-commit 8)))
    #:body
    `(,(header)
      (div
@@ -420,6 +425,8 @@
               (style "font-size: 1.5em; padding-right: 0.4em;"))))
 
   (layout
+   #:title
+   "Comparing derivations"
    #:body
    `(,(header)
      (div
@@ -685,7 +692,11 @@
   (define fields
     (assq-ref query-parameters 'field))
 
+  (define page-header "Package derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -835,7 +846,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Package derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? derivation-changes)
           '(p "No derivation changes")
@@ -950,7 +961,12 @@ enough builds to determine a change")))
     (string-append "?base_commit=" base-commit
                    "&target_commit=" target-commit))
 
+  (define page-header (string-append "Comparing "
+    (string-take base-commit 8) " and " (string-take target-commit 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1042,7 +1058,11 @@ enough builds to determine a change")))
                                           #:optional
                                           base-revision-details
                                           target-revision-details)
+  (define page-header "System test derivation changes")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -1141,7 +1161,7 @@ enough builds to determine a change")))
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "System test derivation changes")
+        (h1 ,page-header)
         ,(if
           (null? changes)
           '(p "No system test derivation changes")
diff --git a/guix-data-service/web/dumps/html.scm b/guix-data-service/web/dumps/html.scm
index 71e69c8..d6d77f9 100644
--- a/guix-data-service/web/dumps/html.scm
+++ b/guix-data-service/web/dumps/html.scm
@@ -22,7 +22,11 @@
   #:export (view-dumps))
 
 (define (view-dumps available-dumps)
+  (define page-header "Database 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..5c5881c 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 (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Recent events")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -256,7 +264,14 @@
              recent-events)))))))))
 
 (define (view-job-queue jobs-and-events)
+  (define page-header
+    (string-append "Queued jobs ("
+              (number->string (length jobs-and-events))
+              ")"))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -267,9 +282,7 @@
         (@ (class "col-sm-12"))
         (a (@ (href "/jobs"))
            (h3 "Jobs"))
-        (h1 "Queued jobs ("
-            ,(length jobs-and-events)
-            ")")))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
@@ -330,7 +343,11 @@
                  jobs-and-events)))))))))
 
 (define (view-job job-id query-parameters log)
+  (define page-header (string-append "Job " job-id))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -339,7 +356,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Job " ,job-id)))
+        (h1 ,page-header)))
       (div
        (@ (class "row"))
        (div
diff --git a/guix-data-service/web/nar/html.scm b/guix-data-service/web/nar/html.scm
index 596d16b..063b091 100644
--- a/guix-data-service/web/nar/html.scm
+++ b/guix-data-service/web/nar/html.scm
@@ -22,7 +22,10 @@
   #:export (view-substitutes))
 
 (define (view-substitutes narinfo-signing-public-key)
+  (define page-header "Substitutes")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -31,7 +34,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-sm-12"))
-        (h1 "Substitutes")
+        (h1 ,page-header)
         ,@(if (canonical-sexp? narinfo-signing-public-key)
               `((h3 "Public key")
                 (pre
diff --git a/guix-data-service/web/package/html.scm b/guix-data-service/web/package/html.scm
index 0d9b078..85b33e9 100644
--- a/guix-data-service/web/package/html.scm
+++ b/guix-data-service/web/package/html.scm
@@ -24,7 +24,10 @@
   #:export (view-package))
 
 (define* (view-package name package-version-with-branches)
+
   (layout
+   #:title
+   (string-append "Package: " name)
    #:body
    `(,(header)
      (div
diff --git a/guix-data-service/web/repository/html.scm b/guix-data-service/web/repository/html.scm
index 88f2632..4bb50db 100644
--- a/guix-data-service/web/repository/html.scm
+++ b/guix-data-service/web/repository/html.scm
@@ -32,7 +32,11 @@
             view-no-latest-revision))
 
 (define* (view-git-repositories git-repositories)
+   (define page-header "Git repositories")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -41,7 +45,7 @@
        (@ (class "row"))
        (div
         (@ (class "col-md-12"))
-        (h1 "Git repositories")))
+        (h1 ,page-header)))
       ,@(map
          (match-lambda
            ((id label url cgit-base-url)
@@ -65,7 +69,11 @@
 (define* (view-git-repository git-repository-id
                               label url cgit-url-base
                               branches-with-most-recent-commits)
+  (define page-header (string-append "Repository " (string-drop url 8)))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -86,7 +94,11 @@
 
 (define (view-branch git-repository-id
                      branch-name query-parameters branch-commits)
+  (define page-header (string-append branch-name " branch"))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -197,7 +209,11 @@
                              branch-name
                              package-name
                              versions-by-revision-range)
+  (define page-header (string-append package-name " on " branch-name))
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -386,6 +402,8 @@
                 (map first derivations-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " package derivations")
    #:body
    `(,(header)
      (div
@@ -636,6 +654,8 @@
                 (map first outputs-by-revision-range))))
 
   (layout
+   #:title
+   (string-append package-name " package outputs")
    #:body
    `(,(header)
      (div
@@ -849,6 +869,8 @@
                                          valid-systems
                                          system-test-history)
   (layout
+   #:title
+   (string-append system-test-name " history")
    #:body
    `(,(header)
      (div
@@ -1016,12 +1038,16 @@
                 '(#f))))))))))))
 
 (define (view-no-latest-revision branch-name)
+   (define page-header "No latest revision")
+
   (layout
+   #:title
+   (string-append page-header " for " branch-name)
    #:body
    `(,(header)
      (div
       (@ (class "container"))
-      (h1 "No latest revision")
+      (h1 ,page-header)
       (p "No latest revision for "
          (strong (samp ,branch-name))
          " branch")))))
diff --git a/guix-data-service/web/revision/html.scm b/guix-data-service/web/revision/html.scm
index 25b79f4..514129b 100644
--- a/guix-data-service/web/revision/html.scm
+++ b/guix-data-service/web/revision/html.scm
@@ -48,7 +48,11 @@
 (define* (view-revision-news commit-hash
                              query-parameters
                              news-entries)
+
   (layout
+   #:title
+   (string-append "Channel News Entries - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -107,7 +111,11 @@
                                 #:key path-base
                                 header-text
                                 header-link)
+
   (layout
+   #:title
+   (string-append "Package: " name " - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -135,7 +143,7 @@
                                          branch-name))))
                   branches)))
           git-repositories-and-branches)
-        (h1 "Package " ,name)))
+        (h1 "Package: " ,name)))
       (div
        (@ (class "row"))
        (div
@@ -169,7 +177,11 @@
                                             #:key header-text
                                             header-link
                                             version-history-link)
+
   (layout
+   #:title
+   (string-append "Package: " name " @ " version " - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -224,7 +236,7 @@
                       (role "button"))
                    "Version history"))
               '())
-        (h1 "Package " ,name " @ " ,version)))
+        (h1 "Package: " ,name " @ " ,version)))
       (div
        (@ (class "row"))
        (div
@@ -471,7 +483,10 @@
                         lint-warning-counts
                         #:key (path-base "/revision/")
                         header-text)
+
   (layout
+   #:title
+   (string-append "Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -548,6 +563,9 @@
        "Home page" "Location" "Licenses")))
 
   (layout
+   #:title
+   (string-append  "Packages - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -756,6 +774,9 @@
      package-description-counts))
 
   (layout
+   #:title
+   (string-append "Packages translation availability - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -835,7 +856,10 @@
                                      query-parameters
                                      #:key (path-base "/revision/")
                                      header-text header-link)
+
   (layout
+   #:title
+   (string-append  "System tests - Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -936,7 +960,11 @@
                                           channel-instances
                                           #:key (path-base "/revision/")
                                           header-text header-link)
+
   (layout
+   #:title
+   (string-append "Channel instances - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1217,6 +1245,9 @@ figure {
                 colours))))))
 
   (layout
+   #:title
+   (string-append "Package substitute availability - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style ,chart-css)
@@ -1254,7 +1285,11 @@ figure {
                                                 #:key (path-base "/revision/")
                                                 header-text
                                                 header-link)
+
   (layout
+   #:title
+   (string-append  "Package reproducibility - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (style "
@@ -1522,6 +1557,9 @@ figure {
     (assq-ref query-parameters 'field))
 
   (layout
+   #:title
+   (string-append  "Package derivations - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1703,6 +1741,9 @@ figure {
       ))
 
   (layout
+   #:title
+   (string-append  "Fixed output package derivations - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -1843,6 +1884,9 @@ figure {
          build-server-urls))
 
   (layout
+   #:title
+   (string-append "Package derivation outputs - Revision "
+    (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2021,7 +2065,10 @@ figure {
                               build-server-options
                               stats
                               builds)
+
   (layout
+   #:title
+   (string-append  "Builds - Revision " (string-take commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2159,6 +2206,9 @@ figure {
      '("Linter" "Message" "Location")))
 
   (layout
+   #:title
+   (string-append "Lint warnings - Revision "
+    (string-take revision-commit-hash 7))
    #:body
    `(,(header)
      (div
@@ -2314,7 +2364,11 @@ figure {
 
 (define (unknown-revision commit-hash job git-repositories-and-branches
                           jobs-and-events)
+  (define page-header "Unknown revision")
+
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2347,13 +2401,16 @@ figure {
                 (strong (@ (class "text-center")
                            (style "font-size: 2em; display: block;"))
                         "Unknown"))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
 
 (define (unprocessed-revision commit-hash job git-repositories-and-branches
                               jobs-and-events)
+  (define page-header "Unknown revision")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
@@ -2375,6 +2432,6 @@ figure {
                        git-repositories-and-branches
                        commit-hash))
                 ,@(view-revision/jobs-and-events jobs-and-events))))
-            `((h1 "Unknown revision")
+            `((h1 ,page-header)
               (p "No known revision with commit "
                  (strong (samp ,commit-hash)))))))))
diff --git a/guix-data-service/web/view/html.scm b/guix-data-service/web/view/html.scm
index 8063e17..33535a9 100644
--- a/guix-data-service/web/view/html.scm
+++ b/guix-data-service/web/view/html.scm
@@ -65,13 +65,15 @@
 (define* (layout #:key
                  (head '())
                  (body '())
-                 (title "Guix Data Service")
+                 title
                  description)
   `((doctype "html")
     (html
      (@ (lang "en"))
      (head
-      (title ,title)
+      (title ,(if title
+               (string-append title " — Guix Data Service")
+               "Guix Data Service"))
       (meta (@ (http-equiv "Content-Type")
                (content "text/html; charset=UTF-8")))
       (meta (@ (name "viewport")
@@ -286,8 +288,7 @@
 (define (index git-repositories-and-revisions)
   (layout
    #:description
-   "The Guix Data Service processes, stores and provides data about Guix over
-time."
+   "The Guix Data Service processes, stores and provides data about Guix over time."
    #:body
    `(,(header)
      (div
@@ -334,7 +335,10 @@ time."
          git-repositories-and-revisions)))))
 
 (define (view-statistics guix-revisions-count derivations-count)
+  (define page-header "Statistics")
   (layout
+   #:title
+   page-header
    #:body
    `(,(header)
      (div
-- 
2.17.1

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 2443 bytes --]


Canan Talayhan <canan.t.talayhan@gmail.com> writes:

> It seems after testing lots of pages this one escaped me since I only
> tested the working case.
>
> Please find the quick fix in the link below.
> https://pastebin.ubuntu.com/p/s7tWyPHZ8F/

Great, that fixes the issue with the revision comparison page.

> I'm looking forward to making another contribution. Could you please
> review it as soon as possible?

I've gone ahead and merged this. I made some changes to the indentation,
I've generally just left that to Emacs, so that's effectively the
indentation style currently. I also tweaked the wording in the commit
message.

As for what to do next, it would be good to start looking at some stuff
that's more related to the project topic.

Part of the revision processing that I believe is quite slow and
hopefully can be improved is populating the package_metadata table. The
relevant lines in the job output look something like this:

  debug: Starting querying the temp_package_metadata
  debug: Finished querying the temp_package_metadata, took 1902 seconds

That output comes from the with-time-logging bit around here [1]. It's a
single query, generated by temp-table-select-query which is taking
around 30 minutes it seems, and I'd hope either the query can be made
faster, or some other faster way of doing what needs doing here can be
found.

1: https://git.savannah.gnu.org/cgit/guix/data-service.git/tree/guix-data-service/model/utils.scm#n333

insert-missing-data-and-return-all-ids is used in a few places, but this
specific issue arises when called from here [2].

2: https://git.savannah.gnu.org/cgit/guix/data-service.git/tree/guix-data-service/model/package-metadata.scm#n373

Making inserting package metadata faster is the overall goal, but I'd
suggest first just trying to reproduce the slow query outside of the
revision processing process. That way you'll be able to look at what the
query is doing and quickly test changes.

The approach I'd recommend is, make yourself a realistic
temp_package_metadata table by populating it with all the
package_metadata entries for a single revision already in your local
database. Then construct and try the slow query, and see how long it
takes, and look at the query plan (run the query with EXPLAIN at the
start).

Do let me know if you have any questions or get stuck, I'll hopefully be
around on IRC, and if I don't respond within a few minutes there, just
email me.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]


Christopher Baines <mail@cbaines.net> writes:

> The approach I'd recommend is, make yourself a realistic
> temp_package_metadata table by populating it with all the
> package_metadata entries for a single revision already in your local
> database. Then construct and try the slow query, and see how long it
> takes, and look at the query plan (run the query with EXPLAIN at the
> start).

Following up on your question on IRC about creating the temporary table,
the code that does this is here [1]. I wouldn't suggest running the
code, just the same SQL it uses, and use relevant values for
temp-table-name and table-name that are appropriate for the
package_metadata table.

1: https://git.savannah.gnu.org/cgit/guix/data-service.git/tree/guix-data-service/model/utils.scm#n313

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Canan Talayhan]

Thanks for your quick response. It helps a lot to me. But still, I
have some confusion about the reproduction steps. As I understand it,
I can reproduce the slow query just using the pure SQL queries without
touching the code for now, right?

Please find my steps below:

1. CREATE TEMPORARY TABLE temp_package_metadata (LIKE package_metadata
INCLUDING ALL)

As we expected, it creates a table but has no data. So I want to
insert some data but colon types are integer and I don't know any
meaningful data for these colons.

2. CREATE TEMPORARY TABLE temp_package_metadata AS TABLE package_metadata

Then I create a copy of the package_metadata.

3. EXPLAIN ANALYZE SELECT * FROM temp_package_metadata
This code generates 155 msec execution time, but I think it should
take more time.

Am I still on the right track?


On Sat, Apr 24, 2021 at 2:39 PM Christopher Baines <mail@cbaines.net> wrote:
>
>
> Christopher Baines <mail@cbaines.net> writes:
>
> > The approach I'd recommend is, make yourself a realistic
> > temp_package_metadata table by populating it with all the
> > package_metadata entries for a single revision already in your local
> > database. Then construct and try the slow query, and see how long it
> > takes, and look at the query plan (run the query with EXPLAIN at the
> > start).
>
> Following up on your question on IRC about creating the temporary table,
> the code that does this is here [1]. I wouldn't suggest running the
> code, just the same SQL it uses, and use relevant values for
> temp-table-name and table-name that are appropriate for the
> package_metadata table.
>
> 1: https://git.savannah.gnu.org/cgit/guix/data-service.git/tree/guix-data-service/model/utils.scm#n313

Re: [Outreachy] - Guix Data Service - Set a more informative page title

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 2682 bytes --]


Canan Talayhan <canan.t.talayhan@gmail.com> writes:

> Thanks for your quick response. It helps a lot to me. But still, I
> have some confusion about the reproduction steps. As I understand it,
> I can reproduce the slow query just using the pure SQL queries without
> touching the code for now, right?
>
> Please find my steps below:
>
> 1. CREATE TEMPORARY TABLE temp_package_metadata (LIKE package_metadata
> INCLUDING ALL)
>
> As we expected, it creates a table but has no data. So I want to
> insert some data but colon types are integer and I don't know any
> meaningful data for these colons.

So, the question to ask here is what data would the temporary table
usually contain during loading data for a revision (when the slow query
happens). I gave more specific information about the data in the
temporary table in my previous email.

> 2. CREATE TEMPORARY TABLE temp_package_metadata AS TABLE package_metadata
>
> Then I create a copy of the package_metadata.
>
> 3. EXPLAIN ANALYZE SELECT * FROM temp_package_metadata
> This code generates 155 msec execution time, but I think it should
> take more time.

I don't see why that query should be slower, but that's also not that
relevant since I think the actual slow query in question here is quite
different.

You can find the query in the code by looking at where it does the
timing for "querying the temp_package_metadata".

I've also got information from the PostgreSQL logs, which gives this as
the query:

  SELECT package_metadata.id, package_metadata.home_page, package_metadata.location_id, package_metadata.license_set_id, package_metadata.package_description_set_id, package_metadata.package_synopsis_set_id FROM package_metadata INNER JOIN temp_package_metadata ON (package_metadata.home_page = temp_package_metadata.home_page OR (package_metadata.home_page IS NULL AND temp_package_metadata.home_page IS NULL)) AND (package_metadata.location_id = temp_package_metadata.location_id OR (package_metadata.location_id IS NULL AND temp_package_metadata.location_id IS NULL)) AND (package_metadata.license_set_id = temp_package_metadata.license_set_id OR (package_metadata.license_set_id IS NULL AND temp_package_metadata.license_set_id IS NULL)) AND (package_metadata.package_description_set_id = temp_package_metadata.package_description_set_id OR (package_metadata.package_description_set_id IS NULL AND temp_package_metadata.package_description_set_id IS NULL)) AND (package_metadata.package_synopsis_set_id = temp_package_metadata.package_synopsis_set_id OR (package_metadata.package_synopsis_set_id IS NULL AND temp_package_metadata.package_synopsis_set_id IS NULL))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]