Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework repo provider #274

Merged
merged 2 commits into from
Dec 29, 2021
Merged

Rework repo provider #274

merged 2 commits into from
Dec 29, 2021

Conversation

starbelly
Copy link
Member

  • align with organizations from mix hex

@starbelly starbelly force-pushed the rework-repo branch 6 times, most recently from 87df0a5 to 22b52fe Compare December 26, 2021 19:58
@starbelly starbelly marked this pull request as ready for review December 26, 2021 19:58
@starbelly starbelly marked this pull request as draft December 27, 2021 21:56
@starbelly starbelly marked this pull request as ready for review December 28, 2021 01:20
handle('GET', [<<"auth">>], Req) ->
respond_with(200, Req, #{});

handle('GET', [<<"orgs">>, <<"foo">>, <<"keys">>], Req) ->
Copy link
Member Author

@starbelly starbelly Dec 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is gross, specifically the hard code, but it'll do for now.

%%
%% Manages the list of authorized hex organizations.
%%
%% Note that all commands that require a `NAME' argument expect a qualified repository name for the
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only thing I'm very unsure of. If we only accept the org part of the repo name, then we have to make some assumptions. In the case you have multiple repos, which one does the org belong to? We can say this is what the -r switch is for, but then we have to say "In this case, you should only specify a parent repository, but in all other cases you can any repository"

If -r only took parent repositories, I'd be ok with just allowing a bare name, but it doesn't, so 🤷 . I mused about switching to that. Basically, it would require that we have a --organization on most tasks.

For example :

rebar3 hex publish -r hexpm --organization foo

or

rebar3 hex organization -r hexpm key foo generate 

or

rebar3 hex owner add rebar3_hex [email protected] -r hexpm --organization foo

So, when I think about that just having -r and requiring the fully qualified repo name for the org task sounds quite good to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that ideally you should be able to reuse the same format we use when declaring dependencies, so that the notation is uniform across publishing and usage when possible?

Copy link
Member Author

@starbelly starbelly Dec 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and what this currently is. That would be hexpm:foo if I'm following correctly. That is, we currently require you to put in {hex, [{repos, [ #{name => <<"hexpm:myorg">>]}]} in rebar.config for fetching private private packages.

In that respect, we should remove support for short hand from publishing. We currently support allowing you to say rebar3 hex publish foo and we will try to find the matching repository, but there's a rub in there, in the case you have an org called foo at two different parent repositories, thus a bug really, no one has run into it yet though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: We currently have no support in rebar3 (AFAIK) for specifying what repository a dep specifically should come from, this was raised in #262.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per this conversation, I will remove support for "shorthand" repo arguments (i.e., -r foo) in the upcoming config provider PR.

@@ -25,8 +26,7 @@

{dialyzer, [
{warnings, [
error_handling,
underspecs
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll come back and deal with this later, there's an issue around it.

- repo becomes organization
- align interfaces with organizations from mix hex
- added rebar3_ex_doc to project_plugins
- add initial docs for rebar3_hex_organization
- add basic supporting tests
@starbelly starbelly merged commit 4a8cb9a into erlef:master Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants