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

Improve logging + Remove Module Proxy Factory artifact dependencies #103

Conversation

asgeir-s
Copy link
Contributor

@asgeir-s asgeir-s commented Jan 2, 2023

  • Make the logging format consistent.
  • Remove leftover Module Proxy Factory artifact dependencies.

cristovaoth
cristovaoth previously approved these changes Jan 2, 2023
Copy link
Contributor

@cristovaoth cristovaoth left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

Minor comment: I noticed that you renamed ModuleProxyFactory to ModuleFactory in the logs. I'll leave it up to you, but since this package has so many Singletons and Factories and etc, I always found that ModuleProxyFactory was a disambiguating name - given what it does, the name is clear and leaves no room for confusing

@asgeir-s
Copy link
Contributor Author

asgeir-s commented Jan 3, 2023

Looks good, thanks.

Minor comment: I noticed that you renamed ModuleProxyFactory to ModuleFactory in the logs. I'll leave it up to you, but since this package has so many Singletons and Factories and etc, I always found that ModuleProxyFactory was a disambiguating name - given what it does, the name is clear and leaves no room for confusing

Thank you. Great suggestion.
The reason for renaming it was that it was sometimes called module factory and sometimes module proxy factory.
However, "module proxy factory" is definitely clearer, so renamed it to that everywhere (except for the filename of src/factory/deployModuleFactory.ts, saving that for the next MAJOR upgrade).

@asgeir-s asgeir-s merged commit 40c4137 into master Jan 3, 2023
@asgeir-s asgeir-s deleted the better-logs_remove-remaining-dependencies-on-ModuleProxyFactory branch January 3, 2023 15:56
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants