GNU bug report logs - #78568
[PATCH 0/5] Synchronize team definitions with Codeberg

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 78568 <at> debbugs.gnu.org
Subject: [bug#78568] [PATCH 5/5] teams: Add ‘sync-codeberg-teams’ action.
Date: Tue, 27 May 2025 12:25:42 +0200

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> That's a sizable amount of code!  Maybe we should embark on a
> guile-codeberg project at some point to have a library for interacting
> with its API.

Agreed.

>> +(define* (codeberg-url items #:key (parameters '()))
>> +  "Construct a Codeberg API URL with the path components ITEMS and query
>> +PARAMETERS."
>> +  (define query
>> +    (match parameters
>> +      (() "")
>> +      (((keys . values) ...)
>> +       (string-append "?" (string-join
>> +                           (map (lambda (key value)
>> +                                  (string-append key "=" value)) ;XXX: hackish
>
> Why is the above considered hackish?  You could use cut to simplify
> out the lambda.

Because it could in theory construct an invalid URL.  It’s OK here
because we can assume that KEY and VALUE don’t contain things like
ampersands, question marks, etc.

>> +(define* (synchronize-team token team
>> +                           #:key
>> +                           (current-teams
>> +                            (organization-teams token
>> +                                                %codeberg-organization))
>> +                           (log-port (current-error-port)))
>> +  "Synchronize TEAM, a <team> record, so that its metadata and list of members
>> +are accurate on Codeberg.  Lookup team IDs among CURRENT-TEAMS."
>> +  (let ((forgejo-team
>> +         (find (let ((name (team-id->forgejo-id (team-id team))))
>> +                 (lambda (candidate)
>> +                   (string=? (forgejo-team-name candidate) name)))
>> +               current-teams)))
>> +    (when forgejo-team
>> +      ;; Delete the previously-created team.
>> +      (format log-port "team '~a' already exists; deleting it~%"
>> +              (forgejo-team-name forgejo-team))
>> +      (delete-team token forgejo-team))
>
> It'd be nicer to update the team, maybe lighter for the endpoint?  What
> was the rationale for going with this more brute force approach?  Was
> the API complex to use, or not available for this?

Updating in general is tricky: you would need to check every aspect of
the team, compute the diff between the current list of members and the
target list of members, etc.  It’s more code and it’s hard to get it
right.

Since the remove/recreate approach doesn’t have undesirable side effects
AFAICS, and since it’s much easier and more reliable, I went for it.

>> +    (format (current-error-port)
>> +            "creating ~a teams in the '~a' organization at Codeberg...~%"
>> +            (length teams) %codeberg-organization)
>
> A bit odd to use current-error-port for this message here; I think the
> other usages of current-error-port were motivated by the fact that the
> other actions of the script were producing copy-pastable output, so you
> could redirect the errors somewhere to avoid corrupting it?  But that's
> not an issue here, so I think we should use current-output-port.

It’s in traditional Unix fashion: logging and error messages go to
standard error.

> Apart from having preferred a more surgical approach to update the teams
> (assuming that would be less overhead for Codeberg), this LGTM.

Thanks!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.