7 ms·
Show HN: A Django code review bot for GitHub pull requests
- rikatee 6y agoDjango Doctor suggests code improvements in your pull requests, so you get better code with no extra effort. What will Django Doctor say about your code? Check now for free at https://django.doctor https://django.doctor If you dig this then you may like to install the bot on your GitHub pull requests https://github.com/marketplace/django-doctor/ https://github.com/marketplace/django-doctor/
- rglullis 6y agoIs this based on some Django linter tool or some kind of plugin I am not aware of?
- rikatee 6y agoNope, all 100% pure custom code. The closest to existing tools are: https://github.com/lamby/django-lint/ https://github.com/lamby/django-lint/ https://github.com/PyCQA/pylint-django https://github.com/PyCQA/pylint-django but- you have to set those up yourself in a CI and update them manually and not have the cool feature like PR integration, "fix it for me" (you would have to fix it yourself like a barbarian), the slick companion website with the beautiful blue progress bar :) With Django Doctor you just click "install on GitHub" and boom, you get updates at no effort, and if you like the suggestion Django Doctor makes you just click commit
- rglullis 6y agoCall me weird, but I would be more willing to support it if it was an open source system that I could control.
- scrollaway 6y agoRan it on a repo of mine, looks like it thinks TextField is the same as CharField in some cases: https://django.doctor/dj-stripe/dj-stripe https://django.doctor/dj-stripe/dj-stripe
- mrlinx 6y agoI'm not one to install these bots on the company repos. Is there a way to run this offline?
- rikatee 6y agoUnfortunately not. It's aimed at SaaS so users gain some convenience but lose a bit of other things :)
- rzodkiew 6y agoI'm probably old fashioned, but I don't understand why would I want something like that as a web-service, rather than something I can run locally (like all the js linters or rubocop).
- cameronh90 6y agoIt's not either/or. We use them in case a developer forgets to run them, or hasn't set their environment up properly or similar.
- raziel2p 6y agoThat's why you make it part of the CI pipeline and require it to pass before merging, making it impossible to forget. The only reason you'd release a tool like this as a github integration and not an open source library (optionally with a CLI tool) is that you want to get money from it - nothing wrong with that, but that is very much an either/or question.
- rikatee 6y agoBlocking the build works well for things that are boolean "correct" or "not correct", but some things are not are just "food for thought". Django doctor points out when something looks like it can be improved, but that's ultimately up to the dev to take the advice. It's "have you thought of..." rather than "you must do this". With the advantage that if the dev likes the advice, all they need to do is click "commit"
- rikatee 6y agoI've been in a few organization with hundreds of repos. It can be a pain to upgrade libraries. In this context, with SaaS you don't need to. But yes, SaaS is not a silver bullet. If you like convenience and getting updates, bug fixes, and quick set up with no overhead then it's for you. If you want to run things yourself then understandable have a great day :)
- 6y ago
- wfleming 6y agoFor anyone who likes the idea but doesn't use Django, check out https://restyled.io/ https://restyled.io/ - it supports a bunch of tools [1]. It's also open source and self-hostable for folks who like the idea but don't trust that automation to a third party. [1]: https://docs.restyled.io/available-restylers/ https://docs.restyled.io/available-restylers/
- marcinzm 6y agoSending all your code to a free third party tools seems risky to me. Any entity without a currently working business model but your data (or access to your systems) is going to be very tempted to abuse that access for money. Maybe not now but down the line after it gets sold to a sketchy Russian company (like a lot of browser extensions).
- richardARPANET 6y agoWalking across the street is also risky.
- marcinzm 6y agoThis is more like walking across the street blindfolded for no good reason. You can run similar tools locally or hosted yourself or open source. edit: Or even a paid alternative as those at least seem to have a business model.
- richardARPANET 6y agoIf you don't value your time, sure.
- rikatee 6y agoin the case of Django Doctor - there is no direct alternative you can run locally. Nothing else does this: - finds Django anti patterns. pretty sweet. - suggests the fix in the PR. One click fix? yes pls. - Check your entire codebase in seconds via https://django.doctor https://django.doctor with no need to install or sign up for anything? hell yeah. - has a fox as a mascot
- michaelmior 6y agoI think it would help to somehow edit the title to add that this is exclusive to Django. Looks cool though! Gave some reasonable suggestions for one of my projects :)
- rikatee 6y agoThanks for the feedback, and glad you found it useful :)
- ninjapenguin54 6y agoEmpty string and none are not the same or in any way redundant. Linters belong in ci. Humans belong in code review. I can actually explain to a human why my text field has a default of none.
- swiley 6y agoI hate things that get this wrong so much.
- rikatee 6y agoI'm sorry swiley :'(
- VWWHFSfQ 6y agoIt's a code-smell in Django, but I agree that this kind of thing shouldn't be "auto-fixed" by a bot. There are perfectly valid reasons why you might want to eschew the convention. > Avoid using null on string-based fields such as CharField and TextField. If a string-based field has null=True, that means it has two possible values for “no data”: NULL, and the empty string. In most cases, it’s redundant to have two possible values for “no data;” the Django convention is to use the empty string, not NULL. One exception is when a CharField has both unique=True and blank=True set. In this situation, null=True is required to avoid unique constraint violations when saving multiple objects with blank values. [0] https://docs.djangoproject.com/en/3.1/ref/models/fields/#null https://docs.djangoproject.com/en/3.1/ref/models/fields/#nul...
- rikatee 6y agoPerhaps I need to clarify "auto fix" - the dev still needs to click "commit" on the suggestion. Given a PR with 3 Django Doctor suggestions, 2 could be ignored and only 1 committed by the dev if they choose.
- eli 6y agoSure, but if it's outside the usual convention it should be explained with a comment. The comment can also include a command to tell the linter to ignore it.
- ivan_ah 6y agoThis is pretty good. Found some "Nullable but not blank=True" errors which were hidden because using mostly as an API, but would have had to fix to use the Django admin or forms. Also suggested reordering of methods within the class, which could be a nice convention to follow for large projects.
- rikatee 6y agoGlad to be of service :)
- bredren 6y agoMight be nice to have option to use this as a service backing a GitHub Action. Sort of an extra checking step to my CI.
- rikatee 6y agoYour wish is granted kind of: https://github.com/marketplace/django-doctor/ https://github.com/marketplace/django-doctor/ It's a github app, rather than an action. Actions still require you to update a yaml file to use. Regarding CI: I don't think Django Doctor should block or fail builds. Django Doctor looks suggests improvements, but it's up to the dev to decide to commit them. Plus, no bot should be commiting changes without dev approval. Hello Skynet!
- bredren 6y agoI saw it is an app, though I don’t see a problem with being able to lint feedback / warnings using an action. For example, I have a project where I pass back CI info to a slack channel so the dev can look at things easy and go to relevant links based on the notification. If I was committing a feature branch and DD had a suggestion it would be fine to know at time of commit. I agree it shouldn’t make it commit changes in this workflow, though. Just point them out.
- zachh 6y agoThis looks really great! Is there a privacy policy?
- rikatee 6y agoThere will be shortly. In summary: - we dont' collect ANY personal data because we don't need ay. we don't even have a database, We can't misuse your persona data if we don't have it! - we don't create any non-essential cookies on browser - your code is deleted as soon as it's checked - the servers are hosted in USA/London
- Rotten194 6y agoIs it possible to use on a private repository? It says "Free for public repositories" but I can't find any information about a non-free option, the github page seems to only list the free tier. Is it not available yet?
- harel 6y agoI've been using it for a while and it works really well. It manages to be helpful and not annoying, which is a fine line to tread.