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 <m...@cbaines.net> wrote: > > > Canan Talayhan <canan.t.talay...@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.