☕ support to track location with pygeoip#42
Conversation
pydanny
left a comment
pydanny
left a comment
There was a problem hiding this comment.
I would remove the whitespace changes from this pull request. During a pull request review, they serve to obfuscate code changes, forcing the reviewer to go over them line-by-line, taking them away from the functionality you are added.
Also, you added a translation file. That should be mentioned in the pull request description.
| user_agent = models.TextField(_("user-agent"), blank=True, null=True) | ||
| timestamp = models.DateTimeField(_("timestamp"), auto_now_add=True) | ||
| path = models.TextField(_("path"), blank=True, null=True) | ||
| username = models.CharField( |
There was a problem hiding this comment.
You shouldn't include whitespace changes in this pull request. Makes it much harder for @dmpayton to review. For reference, whitespace changes should be in its own pull request.
There was a problem hiding this comment.
Oh I see, this because I using python-autopep8.. I'll update for it.. thank you for reviewed..
There was a problem hiding this comment.
I think a command of django-admin makemessages [args] it should automatically reconfigure the number of message, an example from admin_honeypot/admin.py:21 to admin_honeypot/admin.py:26 (whithout changing the previous message).
|
Hi, I have forked this project, included a few PRs, and released a package to PyPI. I would be interested in offering this feature as an option. If you're interested, please open a PR at that repo. Thanks! |
4 participants
No description provided.