cl-github-v3 icon indicating copy to clipboard operation
cl-github-v3 copied to clipboard

Extend API functionality.

Open kingcons opened this issue 11 years ago • 4 comments

Comments welcome.

kingcons avatar Mar 06 '13 18:03 kingcons

Hi Brit,

thank you for the patch. I have one request regarding packages: I see that you import symbols from cl-github into the other packages, but you don't export things. I'd rather like to see it the other way round: Export the API, use qualified names and do not import anything except :cl. Make the package names short, do not repeat the package name in exported symbols, i.e.:

cl-github:define-command github-gist:create

There are cases where the natural name would conflict with a symbol from :cl (i.e. github-gist:get). In such cases, I usually append an asterisk to the name, but I am open to other options.

Does that make sense?

-Hans

On Wed, Mar 6, 2013 at 7:34 PM, Brit Butler [email protected]:

Comments welcome.

You can merge this Pull Request by running

git pull https://github.com/redline6561/cl-github-v3 master

Or view, comment on, or merge it at:

https://github.com/hanshuebner/cl-github-v3/pull/1 Commit Summary

  • Restructure a bit. Flesh out Repo commands.
  • Add basic gists API.
  • Add basic search API.
  • Start using one package-per-file.
  • Add get-user API call.

File Changes

  • A TODOhttps://github.com/hanshuebner/cl-github-v3/pull/1/files#diff-0(18)
  • M cl-github-v3.asdhttps://github.com/hanshuebner/cl-github-v3/pull/1/files#diff-1(6)
  • A gists.lisphttps://github.com/hanshuebner/cl-github-v3/pull/1/files#diff-2(41)
  • M github.lisphttps://github.com/hanshuebner/cl-github-v3/pull/1/files#diff-3(21)
  • A repos.lisphttps://github.com/hanshuebner/cl-github-v3/pull/1/files#diff-4(56)
  • A search.lisphttps://github.com/hanshuebner/cl-github-v3/pull/1/files#diff-5(27)
  • A users.lisphttps://github.com/hanshuebner/cl-github-v3/pull/1/files#diff-6(12)

Patch Links:

  • https://github.com/hanshuebner/cl-github-v3/pull/1.patch
  • https://github.com/hanshuebner/cl-github-v3/pull/1.diff

hanshuebner avatar Mar 06 '13 19:03 hanshuebner

@hanshuebner I'm willing to use qualified names everywhere except define-github-command. Admittedly, I've just never liked the look of toplevel forms that qualify something in another package. Is that an acceptable compromise?

As for not repeating the package name in API exports, I named the calls based on their description in the github API docs. Many calls are named after CL built-ins so I think it's reasonable to suffer through the current naming, not to mention many calls with similar names like https://github.com/redline6561/cl-github-v3/blob/22bb257cc782d66e71efee0edec8ca2290de2e55/repos.lisp#L30. I suppose those could just be renamed "contributors, languages, teams, tags, branches" but those don't seem like very good or descriptive names to me for exported symbols. Tab-completion should ease people's pain here. Thoughts?

kingcons avatar Mar 06 '13 19:03 kingcons

@hanshuebner Here's the patch. I use those symbols all over the place so it just seems to bloat the code to me but I'll do as you like. https://github.com/redline6561/cl-github-v3/commit/dfa7d5bddf3875f59753bd8bc2b273d0b22186b4

kingcons avatar Mar 06 '13 20:03 kingcons

I don't like the importing of define-github-command, really, and I see no reason why top-level names should not be qualified names. I'm not going to make this a red line though.

As for the naming: If the names are just mechanically translated versions of the exported API names from github, then I'm fine with the repetition of parts of the package name in the exported symbols.

As for the symbol repetition:

When I see repetition, I write a macro to remove it. To me, it seems to be time for a github:define-simple-get-command macro:

(github:define-simple-get-command get-branch "/repos/~A/~A/branches/~A" owner repo branch)

-Hans

hanshuebner avatar Mar 06 '13 20:03 hanshuebner