From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:bcc0::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id sI+rI0HYfWDKHQAAgWs5BA (envelope-from ) for ; Mon, 19 Apr 2021 21:21:37 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id YDZkHkHYfWBNDgAAB5/wlQ (envelope-from ) for ; Mon, 19 Apr 2021 19:21:37 +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 3216FE671 for ; Mon, 19 Apr 2021 21:21:37 +0200 (CEST) Received: from localhost ([::1]:58106 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lYZSh-0000Ya-Ul for larch@yhetil.org; Mon, 19 Apr 2021 15:21:35 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48018) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lYZOG-0004Qs-NQ for guix-devel@gnu.org; Mon, 19 Apr 2021 15:17:00 -0400 Received: from mira.cbaines.net ([2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27]:47153) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lYZOE-0000JL-Ei for guix-devel@gnu.org; Mon, 19 Apr 2021 15:17:00 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id C7CB227BC6C; Mon, 19 Apr 2021 20:16:56 +0100 (BST) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id d1486069; Mon, 19 Apr 2021 19:16:56 +0000 (UTC) References: <79d3d2e5c1386b1e162f1ba8380562720131856d.camel@telenet.be> <87tuo7xljp.fsf@cbaines.net> <87lf9ixz5j.fsf@cbaines.net> <87wnsziing.fsf@cbaines.net> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: Canan Talayhan Subject: Re: [Outreachy] - Guix Data Service - Set a more informative page title In-reply-to: Date: Mon, 19 Apr 2021 20:16:53 +0100 Message-ID: <87mttuhyoq.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Received-SPF: pass client-ip=2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-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=1618860097; 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; bh=upeS3v8U0q9F+6p6WHG2UAG2AWGpdDoFP9j5kTKjKAk=; b=bVQIWZL9HIPgw5RTVcZeIf8/fJOfvCTVfg/P1qyEsdagheY+irq0G3WOkYvpBmFBPHVqhY bPAtuKivVS9mcDfwHLPK3Ea2pcxFxPHaxTE5+rSH56oUuwYmcsunC49lLFjyOIIh7LWtu9 OnWW/U013s+p8ZiT59KCd9fXZ0Joj1Pkc7mqi1CshC+L9dloYc57TYWQUfP1I6QKaGid5e RfzzKuS9XpYZ1MxNFqcTobGiys8K90AiliLDBc/lEfos1Ze5vJYO/WYzK01jHWSKJpeQZt XTPHobtXVHi3fOzqBpYCKFgjK/x8U3B2OmQutBfieACpoXVdGWEIZ/IvAfcBOw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1618860097; a=rsa-sha256; cv=none; b=oUjv25jzdx8y7WykR3zcli3jqCQyHxulmJ7aWG+AY2lqoRMICnYyEOq6s5ehC48JaKSqoq aprrw2WrippF5PA170rh56K3MrQSKfKyPnZjCSjCXkxyBvzk8rr5qoWFYSfFLvPWYsYsVd 8GbO4OFw0J+8VRAF4AG+NehEt3jUGpgfajJJqazDeW0PFDDYLoJLomDboXsX9SuA+EcQB/ 3yS/EmwEyhetJ5qVH1GrTyiYzRbMJJqcqXm+YKFBe38GV5W1qFHzIn8pP2n/qVxtjlXqVA LbSQnisLrdPFHI5P4fa6TTPTPTSBT2onjSbQK94g1sVEDw/+eDa+YB/iciAXSQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; 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: -4.54 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; 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: 3216FE671 X-Spam-Score: -4.54 X-Migadu-Scanner: scn0.migadu.com X-TUID: xSQqUYhiLjUQ --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Canan Talayhan 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 wro= te: >> >> >> Canan Talayhan 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-n= ame >> > 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/emac= s/ >> >> 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-servic= e/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-com= mit))) >> (samp ,(string-take base-commit 8) "=E2=80=A6"= )) >> " 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_comm= it)) >> + (h1 ,(let ((base-commit (assq-ref query-parameters 'base_comm= it)) >> >> Why's the @ being removed here? This is still being removed. @@ -435,7 +439,7 @@ " and " (a (@ (href ,(string-append "/revision/" target-com= mit))) (samp ,(string-take target-commit 8) "=E2=80=A6"= ))) - '("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. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmB91yVfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XdOlRAAs9CXtEHRrs910ogP3baA3mO2YAkJBRTD WQDcp03DQXfqurUGmu9c3KNhRApDg6bQPFocboP7aHZct58O8pPtrL1FXC2aenyy TaVoiTK3QkLOKJQYb4+Dhg/4l6Fy0ui8wqwfpNl5KL0SuX2W4kfrby6Oy1mnDa9m P5e5UHKZY5gU5iS3yd3Hzes8rxJlLpWYsTziBOSbAXiWrF6OJSADwDUStA51Fmsp rAPFeBMBEzKgfxlEQTJ1kakjC+ZeNfCtlbUQ+nMpttWN8Zu1C3kjElKxi8Ix1sTX xWpyAiZqq+VSOCIZA3sSc9X+XlKV+mMF5D5bf5ymlxJ93UhElwC9LUrgdMJhf/D/ +0rlj/n2bpiSOxcX+imUDWb0uN4gn2aMHZeMWk2jSHAVRRnJ+Dc5FfdzTlbE1P1A IGKgVulI3SJ5tQr9GIb/vkPwwa9G4zjRSg7dZlm5PggaYRqVmKYRgIsC+oEedOAT TT5Eg/ti0kEvgZnn0Er1l1EjxhE+NBycRfgDjF3niniqWm2C2P9C6+wXtC7o/amz lEU1Sy8JFnSceoClyThNmsb744aVxSa4YeWDEsxL7cmfcFDhvA43KoKI/cieRPbz IBTGCIaMXSD4gwPyQcVo3uU92JQ2IsVCenLikI89O850IjVfg4K5AmEkKQNXxTYQ 81aYp1k0IaQ= =bdGE -----END PGP SIGNATURE----- --=-=-=--