From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:8:6d80::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id aEk7MbvCdWDkrgAAgWs5BA (envelope-from ) for ; Tue, 13 Apr 2021 18:11:39 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id AOweLLvCdWClMAAA1q6Kng (envelope-from ) for ; Tue, 13 Apr 2021 16:11:39 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 927421D53C for ; Tue, 13 Apr 2021 18:11:38 +0200 (CEST) Received: from localhost ([::1]:48642 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lWLdY-0002HU-Ul for larch@yhetil.org; Tue, 13 Apr 2021 12:11:36 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38560) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lWLP9-0002dl-42 for guix-devel@gnu.org; Tue, 13 Apr 2021 11:56:44 -0400 Received: from mail-vs1-xe35.google.com ([2607:f8b0:4864:20::e35]:36628) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lWLP4-0000Zi-9h for guix-devel@gnu.org; Tue, 13 Apr 2021 11:56:42 -0400 Received: by mail-vs1-xe35.google.com with SMTP id k124so8787778vsk.3 for ; Tue, 13 Apr 2021 08:56:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uM06kSNAxqXNZKTRBsLkG9DAdfJIOYQKrh3ofJfhw8I=; b=Xz3y43Ul/l1k58kez7sde8zYtvrs+YHBPqbVkD61NOQRoggY7Vwz6rNDGRK3GzGYRt AiMc/u2CgdyR1g5rE3GZGg4moFosLlaGRdSakf3ynlv/9PsHUWHWDQ5jWFC+uj4dDi9d C3f7IgTb6C9mxEvqd7QDOXVPP4vSrSszjITBmxwMvMl8Ckge2p3Bp98DLlZnvyjpZh7i Ca8KQpag8owR9S5fODFvJptR8XNaAMvYzsMKkWKe5t+GOxeQtTwTzYKoElGf/S2G7dFx cVCexdRiFKic3E0rR3KGBHxfWY6z+YgZlMmI1qSMozgD/5nkWuX9cgMnLXFHiinE+i4J MYqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uM06kSNAxqXNZKTRBsLkG9DAdfJIOYQKrh3ofJfhw8I=; b=k8rLX8ccVt/7lYJNh8M1xWmKMPeXRzIJDxpVPFvGJAcqlfFbWkx9GTcVJMPN0nTUyS V3lv0cD5OCfZXf7rJFsIS2a6XP9DtU1lQ4z+7+eE3KZYlrQLWhEo/NmKersTwd5e9NNO XCrgFV88k3GmVCke7dze20d2qQiU+BuS2bD6JiBarNYp5jacgypXDRu3CbId29jsQAwO dtZRoNRHIIpMJuYRDSu6TV640LqskA94Rv+x/2kznnqtBA82WOmjBulS441tkSc5Y3zz S68ZhWbvjQtZ1hWOIg+pAiaChdqaprIHoQiVaiHZBHH0jcL97voPS1JKByCwiRC4EkLj 0EGg== X-Gm-Message-State: AOAM531aWBO35/Qzto1v7qFTCYvxMvrghm1LmhDFtxMLdTSYbJFBmmZM w+s26Iywmhbmie94HytlB+hFvnKnfU6GVcDgVEQ= X-Google-Smtp-Source: ABdhPJwIqpGZUAqaa6j0v5HTKsnv0XmY/ySWFVIOvT0BUmrW6qAcNei2a2AFKgd7orz8sxsXyH+LJbS7mW4krStg5J8= X-Received: by 2002:a67:3252:: with SMTP id y79mr22747969vsy.26.1618329397315; Tue, 13 Apr 2021 08:56:37 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Canan Talayhan Date: Tue, 13 Apr 2021 18:56:00 +0300 Message-ID: Subject: Re: [Outreachy] - Guix Data Service - Set a more informative page title To: Maxime Devos Content-Type: multipart/alternative; boundary="000000000000db4cdf05bfdcac85" Received-SPF: pass client-ip=2607:f8b0:4864:20::e35; envelope-from=canan.t.talayhan@gmail.com; helo=mail-vs1-xe35.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: guix-devel@gnu.org Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1618330299; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=uM06kSNAxqXNZKTRBsLkG9DAdfJIOYQKrh3ofJfhw8I=; b=dWUPWtAcYZVjWcw8Mo+bb+pwQAU+GBSDIUy5caHal+9xx9Gr2TX4qDfi17WYM7EDBofSLB +qBEHYJa1/cbQjGsLlQzCBtLTFj75umViK04EulohGAqEHpFaFMUEt6T/WekihAFlYXyGq 8jF8RAjjWpFJ97AEHQ4mwC+MxB7Zzmgi1kyDLuqV8m/uas34dMGNdqHvWOdTgWYYq3BPeR 6FftMVYcFZo3hm6Sp6iLY0GjwPpaRVDyiq895tM7je5A4is9NAfOs0i3I5LvYs440sD9dP pkaFGCGCrSNfn3aFgPlKmfw+jla8DRTC4SqYLlCXBu1I+sK1OGvOPsN4LUqRng== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1618330299; a=rsa-sha256; cv=none; b=ChX103sKK0Sa46C6VGk7pAcbE/kzj+cf1LVJIOY82uA/Mu1zyorL5KZCovutd6kTQcCrKN K+VSNlwpIYPT2hp27bg9vSSKONnpVXw6mlXwksy0vZlaj8bH/jOjqwcnC86LVpNJAHOb4z htf4lrCPgVNlYhY+cSC631+vxUw/S6yPXPCHyhjPHZog0KGDSY0Gl63dPR0wiVh5tuki+i 0eGwjUZuwoTfB2rd9XlTC36bjLPIkI9gUSxdwo5BqJXV2W5P+3f/lGEnbOW1DL763hAdmy Oxn1u/McauUgMKYm9SkDAEdJRB42fwzYaqM3Ox/m6GBhQeJw8nxVwblQChORag== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=Xz3y43Ul; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Spam-Score: -1.14 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=Xz3y43Ul; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Queue-Id: 927421D53C X-Spam-Score: -1.14 X-Migadu-Scanner: scn0.migadu.com X-TUID: e7SdUVhDrQuD --000000000000db4cdf05bfdcac85 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Maxime, Is this =E2=80=98introductory task=E2=80=99 publicly available? If so, cou= ld 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 = =E2=80=92 > or the =E2=80=94 > em dash, I always forgot which is correct), instead of the hyphen-minus -= . > (Resource: ). > It's a good catch : ) done. You could bring the 'if' inside: > `((title ,(if title > (string-append title " =E2=80=92 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 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 =E2=80=98introductory task=E2=80=99 publicly available? If so, c= ould 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 = =E2=80=92 > or the =E2=80=94 > em dash, I always forgot which is correct), instead of the hyphen-minus -= . > (Resource: ). > > > `((title "Guix Data Service"))) > > ``` > > You could bring the 'if' inside: > `((title ,(if title > (string-append title " =E2=80=92 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 > --000000000000db4cdf05bfdcac85 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Maxime,

Is this =E2=80=98introductory task=E2=80=99 publ= icly available?=C2=A0 If so, could you post a link?
I'm not up-to-date with Outreach= y.

=
It's not available under the Official Outreachy page if you&= #39;re not logged in.


Typographical nitpick: I would use a proper = dash here (the figure dash =E2=80=92 or the =E2=80=94
em dash, I always forgot which is correct), instead of the hyphen-minus -.<= br> (Resource: <https://en.wikipedia.org/wiki/Dash#Figu= re_dash>).

It's a good catch : ) done.

You could bring the 'i= f' inside:
=C2=A0 `((title ,(if title
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (string-append titl= e " =E2=80=92 Guix Data Service")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "Guix Data Ser= vice")))


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

After sending the patch I've tur= ned the patch like below.
=C2=A0 =C2=A0 =C2=A0 (title= ,(if title
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 `,(string-append t= itle " - Guix Data Service")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 '("Guix Data Service")))

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

We're generating titles using the available h1 value to prevent d= uplication.

Adding space between the id and job co= uld 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 Dev= os <maximedevos@telenet.be= > wrote:
On T= ue, 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 t= he introductory task for
> Guix Data Service.

Is this =E2=80=98introductory task=E2=80=99 publicly available?=C2=A0 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 &quo= t;Guix Data Service"
>
> I've created a patch for the "Jobs" page. If it looks go= od for everyone then I can proceed with
>=C2=A0 other applicable pages.
>
> Now, I'm working on the title part of the code snippet below to ma= ke it more elegant.
>
> ```scm
> (define* (layout #:key
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (head &#= 39;())
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (body &#= 39;())
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 title >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 descript= ion)
>=C2=A0 =C2=A0`((doctype "html")
>=C2=A0 =C2=A0 =C2=A0(html
>=C2=A0 =C2=A0 =C2=A0 (@ (lang "en"))
>=C2=A0 =C2=A0 =C2=A0 (head
>=C2=A0 =C2=A0 =C2=A0 =C2=A0,@(if title
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0`((title ,(string-appen= d title " - Guix Data Service")))

Typographical nitpick: I would use a proper dash here (the figure dash =E2= =80=92 or the =E2=80=94
em dash, I always forgot which is correct), instead of the hyphen-minus -.<= br> (Resource: <https://en.wikipedia.org/wiki/Dash#Figu= re_dash>).

>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0`((title "Guix Dat= a Service")))
> ```

You could bring the 'if' inside:
=C2=A0 `((title ,(if title
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (string-append titl= e " =E2=80=92 Guix Data Service")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "Guix Data Ser= vice")))


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

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

+=C2=A0 (define page-header"Jobs")

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

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

IIRC, job ids are integers, so this would result in titles
like "Job1234"?=C2=A0 Titles like "Job 1234" would be b= etter.

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
--000000000000db4cdf05bfdcac85--