-
Notifications
You must be signed in to change notification settings - Fork 535
feat: Maintain a global configuration instance #207
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
Conversation
vdusek
left a comment
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.
What about a simple test case testing if it's called multiple times it always returns the object with the same ID? Or it's not necessary?
Good point. Also we'll need to reset the global instance before each test, I guess. |
vdusek
left a comment
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.
Just a thought on the naming...
I suggest using get_global instead of get_global_configuration. Since the method serves as constructor for the Configuration class - including "configuration" in the method name seems redundant. Leaving it up to you.
Otherwise, LGTM.
I was ready to tell you that it's named that way because of JS crawlee, but there it's called |
No description provided.