-
Notifications
You must be signed in to change notification settings - Fork 0
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
Timezone utils #194
Timezone utils #194
Conversation
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.
I've only left the one comment for now because I think it'll be a pretty big change. I'll review again once you've tackled that
config/settings/base.py
Outdated
@@ -47,6 +47,7 @@ | |||
"rp_interceptors", | |||
"rp_yal", | |||
"randomisation", | |||
"timezone_utils", |
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.
Can you change the app to be msisdn_utils
? I think an app where we can put small tools for handling phone numbers will be more useful in the long run than a timezone app
This change will probably require quite a bit of shifting things around I'm afraid
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.
One question about libraries removed from setup.cfg
Otherwise it looks great 🍰
@@ -16,4 +16,4 @@ line_length = 88 | |||
multi_line_output = 3 | |||
include_trailing_comma = True | |||
skip = ve/,env/ | |||
known_third_party = boto3,celery,dj_database_url,django,environ,freezegun,hashids,json2html,kombu,moto,phonenumber_field,pkg_resources,prometheus_client,pytest,raven,recommonmark,requests,responses,rest_framework,sentry_sdk,setuptools,six,temba_client | |||
known_third_party = celery,dj_database_url,django,environ,freezegun,hashids,json2html,jsonschema,kombu,phonenumber_field,phonenumbers,pkg_resources,prometheus_client,pytest,pytz,raven,recommonmark,redis,requests,responses,rest_framework,sentry_sdk,setuptools,six,temba_client |
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.
@MatthewWeppenaar did you update these or did an automated action update them?
I'm concerned about the removal of some of the libraries (especially boto3) so just want to check if something prompted you to do it or if it's automated.
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.
Im not sure why this was removed, i didn't manually edit that line in setup.cfg
so it must have been removed when i ran a function in the command line🤷♂️ Shall i add that line back?
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.
ok no that's fine. Just wanted to understand the reason behind it but if it was an automated process then it was probably removed in a previous change and the setup.cfg file wasn't updated.
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 description provided.