Initial work on hex_api #11
Conversation
test/hex_api_tests.erl
Outdated
|
|
||
| get_package_test() -> | ||
| {ok, Package} = hex_api:get_package("ecto", ?OPTIONS), | ||
| #{<<"name">> := <<"ecto">>, <<"releases">> := _} = Package, |
There was a problem hiding this comment.
another option is to return atom keys so that it's easier to work with, mimics registry functions (also atom keys), and we could document all fields as a type. wdyt @ericmj?
somewhat related, should it be hex_api:get_package("ecto") or hex_api:get_package(<<"ecto">>) here and in other places? I'm starting to think the latter would be better because we almost always return binaries not charlists.
There was a problem hiding this comment.
Atom keys sound good. We should binaries over charlists as much as possible.
|
@ericmj I still need to add docs but would appreciate feedback on the implementation in the meantime. |
src/hex_api.erl
Outdated
| {downloads, [week, recent, day, all]}, | ||
| inserted_at, | ||
| updated_at | ||
| ], |
There was a problem hiding this comment.
Are the fields for string to atom conversion? I wonder if we should use strings instead so that we don't have to update this library for each new field the API adds.
There was a problem hiding this comment.
#11 (comment) :) I wanted atom keys and chose whitelist approach as opposed to blacklist approach for 2 reasons: it nicely handles deprecated fields and different shapes: downloads shape is known but links is not etc.
I am aware of increased maintenance burden of adding new fields. Hope its not too pretentious but I consider Hex to be mostly complete :) so I dont think this will be a huge issue in practice but its definitely not ideal. That being said, If we stick to it I gotta feeling I will regrey it the next time we do happen to add a new field, but let me sit on it for a couple days anyway :)
There was a problem hiding this comment.
Thanks for the reminder 😀. Sounds good.
There was a problem hiding this comment.
I've decided to revert these changes. The reason is when hex_erl is released and used by build tools, when we add a new field in hexpm we'll need to update hex_erl and then bump both hex & rebar which seems too much.
I'd still like to use atom keys here (and in tarball metadata!) but maybe will use blacklist approach later; keeping as strings for now.
sorry for going back'n'forth 😅
There's more to be done in other PRs, see updated roadmap: #6.