-
Notifications
You must be signed in to change notification settings - Fork 145
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
Feat/get registry from uris #592
base: main
Are you sure you want to change the base?
Feat/get registry from uris #592
Conversation
🦋 Changeset detectedLatest commit: cda809b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
.filter((uri) => !!uri) | ||
.map((uri, index) => { | ||
const childLogger = registryLogger?.child({ uri, index }); | ||
if (isHttpsUrl(uri)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fyi we are working on a HTTP registry #464, we we probably need a different way of identifying that it is supposed to be a github registry. I personally was in favor of using protocol prefix for that, but i got overruled
const githubUrl = 'https://github.com/hyperlane-xyz/hyperlane-registry'; | ||
|
||
describe('getRegistry', () => { | ||
it('creates MergedRegistry with FileSystemRegistry for local path', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional Nit: For all these tests, i wonder if its nice to just have a mapping of [urls] => [expectedRegistry]
and then just run deep equal or something like that?
'@hyperlane-xyz/registry': minor | ||
--- | ||
|
||
Add getRegistry and getMergedRegistry functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer have getMergedRegistry
in here yeah?
Description
This PR introduces and refactors registry utility functions:
getRegistry
utility that:MergedRegistry
containing appropriate registry instances (Github/FileSystem) based on URIsPROXY_DEPLOYED_URL
constant for standardized proxy endpointBackward compatibility
Yes
Related issues
hyperlane-xyz/hyperlane-monorepo#5403
Testing
The changes have been extensively tested through unit tests covering:
All functionality maintains compatibility with existing use cases while providing a more streamlined API.