Skip to content

Created an option to find modules via identifiers#327

Closed
saxenism wants to merge 8 commits intodevfrom
saxenism/issue-317-same-name-modules
Closed

Created an option to find modules via identifiers#327
saxenism wants to merge 8 commits intodevfrom
saxenism/issue-317-same-name-modules

Conversation

@saxenism
Copy link
Contributor

@saxenism saxenism commented Aug 15, 2023

As a fix for the issue mentioned by the auditors in issue #317 regarding the findModuleAddressInOrchestrator not designed to handle the cases where multiple modules with the same name are being used, I have made a few changes to findModuleAddressInOrchestrator.

Since the URL of a module is much less likely to be the same as other modules, I have replaced the query string from the module title to the module URL.

And since, it is technically possible to have multiple modules with the same URL, i have also given an option to search for module address using a unique module identifier.

@FHieser
Copy link
Contributor

FHieser commented Aug 17, 2023

Where tests

Copy link
Contributor

Choose a reason for hiding this comment

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

The tests are missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

The tests are no longer missing 👉👈

@saxenism
Copy link
Contributor Author

saxenism commented Aug 18, 2023

Considering this comment:

Added the documentation for dependencyData.

The dependencyData now expects 3 fields:

  1. bool hasDependency: True if dependencies are expected, false otherwise
  2. string[] dependenciesURLs: URL of the modules that can be counted as dependencies
  3. bytes additionalData: Any additional data that you might want access to, inside of init2.

@marvinkruse marvinkruse self-requested a review August 29, 2023 09:11
@FHieser FHieser changed the base branch from main to dev September 27, 2023 08:42
@FHieser
Copy link
Contributor

FHieser commented Dec 7, 2023

The changes will be implemented with the #371 PR

@FHieser FHieser closed this Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants