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

Add hex.registry add Command #931

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aj-foster
Copy link

This PR supersedes #920. See that PR for previous discussion.

In this PR, we add a mix hex.registry add command to the existing mix task. The purpose of this command is to allow private registry maintainers to add one or more packages to an existing registry without necessarily having all of the package tarballs present.

At the time or writing, this PR is a minimal effort for the purpose of discussion. We have an opportunity to make larger changes to the hex.registry task to support future growth, if desired.

Some topics to consider:

  • Do you have any recommendations for error messages when, for example, someone attempts to add a non-existent tarball?
  • Should we support adding a package that matches one already in the registry?
  • Should we create separate (and more granular) functions for reading and writing local registries, for the purpose of reuse as we add additional commands in the future (e.g. remove, retire)?
  • Should we provide test setup helpers, such as create_local_registry or even an in_local_registry macro?

With your guidance, I'm happy to contribute towards the right solution for the long term.

lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
src/mix_hex_registry.erl Outdated Show resolved Hide resolved
lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
names =
for {name, %{updated_at: updated_at}} <- versions do
%{name: name, updated_at: updated_at}
end
Copy link
Member

Choose a reason for hiding this comment

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

Enum.map is preferred here since it will error if the match fails.

Copy link
Author

Choose a reason for hiding this comment

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

Should we change build to match? If desired, I can include changes to build in this PR or save them for a follow-up PR.

lib/mix/tasks/hex.registry.ex Outdated Show resolved Hide resolved
@ericmj
Copy link
Member

ericmj commented Nov 2, 2022

Hi @aj-foster! What's the status of this PR?

@aj-foster
Copy link
Author

Hi @ericmj I still need to respond to your comments and make changes based on the merged changes to hex_core. Apologies for not cleaning this up sooner.

@ericmj
Copy link
Member

ericmj commented Nov 7, 2022

@aj-foster No worries, just checking in to get the status. I will leave the PR open and you can get to it if you get the time. Thank you! 💜

@aj-foster aj-foster force-pushed the aj/feat/registry-add branch from d3262e6 to cd00ed8 Compare November 12, 2022 04:56
@aj-foster
Copy link
Author

aj-foster commented Nov 12, 2022

Hi @ericmj — Made some updates to this PR. Hopefully I re-understood everything correctly, since it's been a while. 🙂 Just a few comments in the above threads to make sure I'm on the right track.

@ericmj ericmj force-pushed the main branch 2 times, most recently from 24c6c48 to 0edaffc Compare February 2, 2023 01:32
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.

3 participants