The repos subapi is very inconsistent. I want to fix that but would
first like to know if you agree and whether I would have to make my
changes backward compatible.
I identified the presence of the gh-repos-repo-stub class as the main
cause of most of the inconsistencies. I guess that the reason this
class was introduced was that it requires less boilerplate code to
create a stub as compared to creating an actual (and "complete") repo
object when making api calls that don't actually require the values of
any of the additional slots that gh-repos-repo objects have.
So to save the user from writing all that code when it is not needed
the gh-repos-repo-stub was added. Of course an incomplete
gh-repos-repo object could instead be used in all cases where a
gh-repos-repo-stub is needed but adding the latter has the benefit
that one can easily tell whether some method works with a incomplete
repository representation or not.
Or maybe your intention was that gh-repos-repo objects should never be
created using it's constructor directly but always be created from
responses from github.
However, in some cases not even an gh-repos-repo-stub is required: a
simple string is enough. So now we have methods that require (1) a
gh-repos-repo, (2) a gh-repos-repo-stub, or (3) a string ("repo-id")
as second argument. And which of these are actually required by some
functions appears to be completely arbitrary.
Why
- gh-repo-repo-get(api repo-ID &optional user) but
- gh-repo-repo-update|rename(api repo-STUB &optional user ...)
Both functions default to doing something with a named repository of
the user returned by gh-api-get-username; and can be told to instead
do there thing to the repository by the same name of the user given
with the optional USER argument. So why does gh-repos-repo-update
expect a gh-repos-repo-stub as argument when just a string is enough,
but gh-repo-get is happy with just the string?
Why gh-repos-repo-contributors|languages|... (api REPO) ?
All of these functions need to know the owner (USER) and the name
(REPO-ID) of the repository. So why does one have to write
(gh-repos-repo-contributors
api
(gh-repos-repo "repo" :name "<NAME>"
:owner (gh-user "user" :login "<OWNER>")))
When just
(gh-repos-repo-contributors "<NAME>" ["<OWNER>"]) or
(gh-repos-repo-contributors "[<OWNER>]/<NAME>")
would be enough information?
Why does gh-repos-repo-get work like that but these functions do not?
Because we can do this?:
(gh-repos-repo-contributors
api
(gh-repos-repo-get api "<NAME>" ["<OWNER>"]))
I still haven't figured out whether your intention was to only create
gh-repos-repo objects from data returned by github, or if you expected
that such objects would be created from scratch.
Regardless I think it makes no sense (1) to make an api call just to
get some information we already have and (2) that the alternative of
creating an repo object using the constructor currently requires way
to much code.
So I propose that
- the gh-repos-repo-stub class be removed
- all functions that require OWNER and NAME be changed to expect an
gh-repos-repo object
- there be an easy way to create such a repo object
The constructor should be changed to accept such input:
(gh-repos-repo "OWNER/NAME")
=pp=> (gh-repos-repo
:full-name "OWNER/NAME"
:name "NAME"
:owner (gh-repo :login "OWNER"))
(gh-repos-repo "NAME") or
(gh-repos-repo "/NAME")
=pp=> (gh-repos-repo
:full-name "GH-API-GET-USERNAME/NAME"
:name "NAME"
:owner (gh-repo :login "GH-API-GET-USERNAME"))
All functions (of the repos subapi) should never use
gh-api-get-username and instead rely on the gh-repos-repo object
always containing the OWNER information. If OWNER is the logged in
user the second form above can be used. This way gh-api-get-username
is still used effortlessly but now when the repo object is created, as
opposed to when some method is called.
Also note that the functions which only require just the OWNER and
NAME (almost all functions in the repos subapi) could instead be
changed to just expect two strings.
Some of these functions currently expect just that (two strings), some
a stub and OWNER, some just a stub (and some of these optionally OWNER
if different from the USER in the stub), and some an actual repo
object. :-\ So switching to expecting two strings would be an
improvement, however in some cases we might already have a repo
object around and requiring two strings as arguments would again lead
to boilerplate - this time in a different situation:
(gh-repos-repo-something (oref repo-object :name)
(oref (oref repo-object :name) :login))
It would be best if all methods dispatched on either a repo or two
strings but that is not possible because eieio only dispatches on the
first argument which is used for the api object. (Well we could do
the dispatching ourselves, or move the API argument later in the
argument list... iiks.)
Of course the
... (oref repo-object :name)
(oref (oref repo-object :name) :login) ...
part still appears somewhere, that is inside the methods but that
isn't different now. This repetition could be reduced with a
gh-format function that could hide that aspect away. But that is
something to worry about later and not necessary at all.
(gh-format "/repos/%u/%n" REPO) ; using format-spec
There are a few related issues I would like to address at the same time:
- gh-reposrepo-new|update should lose the CAPS argument, instead:
x
(gh-repos-repo-update (gh-repos-repo "/NAME")
:issues t
:wiki nil
;; :description ??? -> unbound: keep current value
:master-branch "next" ;; currently missing
)
(oset existing-repo-object :description "bla bla")
(gh-repos-repo-update existing-repo-object)
This "changes" :issues, :wiki... to what we already have. That's not
a problem: we already do that for :name.
It's unfortunate that github uses different end-points for user and
org repositories. So:
- gh-repos-repo-new: the ORG argument should be a boolean (because
the owner name is already known due to the REPO argument but the fact
that the owner is a organization is not).
- probably keep gh-repos-org-list (Alternatively rename
gh-repos-user-list -> gh-repos-list and give it a ORG-P argument.
But I don't think that's a good idea.)
So what do you think?
To summarize my argument: Currently what kind of argument a method in
the repos api expects is fallows no obvious logic. Most functions
just need to know the name and owner of a repository so we could
change these functions to just expect two string. But because we
might already have an object in memory that could be reused we should
instead change all functions to expect a gh-repos-repo object and make
it effortless (= noise-less) to create such an object using the
constructor.
Best regards,
Jonas
Ps: Note that I can not implement and test all this right away. But
it would certainly motivate me if I knew that you agree with the
general direction I have outlined.
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent