From: Christopher Baines <mail@cbaines.net>
To: Luciana Lima Brito <lubrito@posteo.net>
Cc: guix-devel@gnu.org
Subject: Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
Date: Fri, 16 Apr 2021 00:19:46 +0100 [thread overview]
Message-ID: <87o8efxhil.fsf@cbaines.net> (raw)
In-Reply-To: <20210415130947.7387a546@lubrito>
[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]
Luciana Lima Brito <lubrito@posteo.net> writes:
> On Thu, 15 Apr 2021 09:46:12 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
> Hi,
>
>> I'm not quite sure how to apply this, I'd suggest using git
>> format-patch to generate the file next time as I think there would
>> normally be some metadata along with the diff.
>
> I tried using git format-patch and I got 7 patches from my 7 commits,
> then I generate a single patch output, which is attached.
> The last commit before my modifications is this:
> 410f58cb43f083623885a430700c6818a187cadc
Ok, I looked at the overall diff, and it looks to me like these changes
should probably be one commit.
>> I would consider whether it's useful to have all these let blocks, and
>> whether here is the right place for them.
>
>> Taking a binding like outputs, it's only used in a later let. You can
>> do something like this (with let*) to remove the need to have
>> multiple let blocks.
>
>> Also, since matched-outputs is only used when rendering the JSON, I'd
>> move all the bindings that are only used for the JSON output within
>> that part of the case statement, so that it's clearer that they only
>> apply to that bit of the code.
>>
>> Does that make sense?
>
> I did it, I used the let* and this helped a lot. I also moved
> everything into the case branch of the json.
>
>> I'm not sure what revision here referrs to.
>
> It was a placeholder, but now I removed it.
>
>> I hope that helps, just let me know if you have any questions,
>
> The function get-derivation-data does not depend on anything, don't you
> think it goes better in another place outside render-compare/derivation?
On the get-derivation-data function, I wouldn't use the same function to
process the inputs, outputs and sources. The data for each is different,
so I would separate the code as well.
To avoid having to call a procedure three times, on the base, target and
common items, I'd consider following the same pattern in the HTML
generating code, map over a list of the lists, so something like:
(map (lambda (name data)
(cons name
(match data
((name path hash-alg hash recursive)
...))))
'(base target common)
(list (assq-ref outputs 'base)
(assq-ref outputs 'target)
(assq-ref outputs 'common)))
Does that make sense?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]
next prev parent reply other threads:[~2021-04-15 23:20 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-14 19:48 Outreachy - Guix Data Service: implementing basic json output for derivation comparison page Luciana Lima Brito
2021-04-15 8:46 ` Christopher Baines
2021-04-15 16:09 ` Luciana Lima Brito
2021-04-15 23:19 ` Christopher Baines [this message]
2021-04-16 15:07 ` Luciana Lima Brito
2021-04-16 15:47 ` Christopher Baines
2021-04-16 18:46 ` Luciana Lima Brito
2021-04-16 19:17 ` Christopher Baines
2021-04-16 22:47 ` Luciana Lima Brito
2021-04-17 8:40 ` Christopher Baines
2021-04-17 12:48 ` Luciana Lima Brito
2021-04-17 13:11 ` Christopher Baines
2021-04-17 14:08 ` Luciana Lima Brito
2021-04-17 17:45 ` Christopher Baines
2021-04-18 13:12 ` Luciana Lima Brito
2021-04-18 13:19 ` Luciana Lima Brito
2021-04-18 16:34 ` Christopher Baines
2021-04-18 19:12 ` Luciana Lima Brito
2021-04-19 8:26 ` Christopher Baines
2021-04-19 14:04 ` Luciana Lima Brito
2021-04-19 20:20 ` Christopher Baines
2021-04-19 20:56 ` Luciana Lima Brito
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87o8efxhil.fsf@cbaines.net \
--to=mail@cbaines.net \
--cc=guix-devel@gnu.org \
--cc=lubrito@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).