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

Refactor cmd/metadata.go for clarity #5908

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Nov 6, 2024

Description

This PR is best reviewed commit by commit:

  • 243e0a7 moves repoSlug parsing into {owner}/{name} into it's own type.
  • 1bd3c23 makes mainSpec a file local variable.
  • 0c8d46c moves schema fetching into it's own set of functions.
  • 4b50e40 rewrites the boolean logic for determining what is a component vs a native provider vs neither.
  • fbaae16 simplifies the publisher logic into a switch statement.

☝️ are pure refactors, and will not change behavior.

  • 9e6554f changes the GH repo slug to require input in the form a/b where both a and b are non-empty. Previously / would have been accepted.

@t0yv0
Copy link
Member

t0yv0 commented Nov 6, 2024

Would it be possible to put the codebase under test before touching it? I do not see any test files at all around this small codebase. It does not look like a lot is happening here so perhaps the testing itself does not need to be particularly complicated.

Copy link

github-actions bot commented Nov 6, 2024

Your site preview for commit 9e6554f is ready! 🎉

http://registry--origin-pr-5908-9e6554fc.s3-website.us-west-2.amazonaws.com/registry.

@iwahbe
Copy link
Member Author

iwahbe commented Nov 7, 2024

Would it be possible to put the codebase under test before touching it? I do not see any test files at all around this small codebase. It does not look like a lot is happening here so perhaps the testing itself does not need to be particularly complicated.

Yes. I have rebased on top of f99cca5, which adds tests for this functionality.

Copy link

github-actions bot commented Nov 7, 2024

Your site preview for commit 3963bac is ready! 🎉

http://registry--origin-pr-5908-3963bac1.s3-website.us-west-2.amazonaws.com/registry.

Copy link

github-actions bot commented Nov 7, 2024

Your site preview for commit 4e20103 is ready! 🎉

http://registry--origin-pr-5908-4e20103e.s3-website.us-west-2.amazonaws.com/registry.

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