gentoo client support #652
Conversation
![code-review-doctor[bot]](https://avatars.githubusercontent.com/in/83430?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good. Worth considering though. View full project report here.
furlongm
force-pushed
the
gentoo-client-support
branch
4 times, most recently
from
e894daf to
c73a5b8
Compare
furlongm
force-pushed
the
gentoo-client-support
branch
from
c73a5b8 to
4a9a140
Compare
furlongm
force-pushed
the
gentoo-client-support
branch
from
4a9a140 to
9160512
Compare
There was a problem hiding this comment.
Looks good. Worth considering though. View full project report here.
furlongm
force-pushed
the
gentoo-client-support
branch
from
9160512 to
90f9408
Compare
furlongm
force-pushed
the
gentoo-client-support
branch
from
90f9408 to
51cb45b
Compare
furlongm
force-pushed
the
gentoo-client-support
branch
from
51cb45b to
d3b29a4
Compare
furlongm
force-pushed
the
gentoo-client-support
branch
from
d3b29a4 to
1efefc9
Compare
There was a problem hiding this comment.
Looks good. Worth considering though. View full project report here.
| choices=PACKAGE_TYPES, | ||
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) |
There was a problem hiding this comment.
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Read more.
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) | ||
| category = models.ForeignKey(PackageCategory, blank=True, null=True, on_delete=models.SET_NULL) |
There was a problem hiding this comment.
Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. Read more.
| name = models.CharField(max_length=255) | ||
| version = models.CharField(max_length=255) | ||
| epoch = models.CharField(max_length=255, blank=True, null=True) | ||
| release = models.CharField(max_length=255, blank=True, null=True) | ||
| arch = models.CharField(max_length=255) | ||
| packagetype = models.CharField(max_length=1, blank=True, null=True) | ||
| category = models.CharField(max_length=255, blank=True, null=True) |
There was a problem hiding this comment.
| category = models.CharField(max_length=255, blank=True, null=True) | |
| category = models.CharField(max_length=255, blank=True, default='') |
Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).
furlongm
force-pushed
the
gentoo-client-support
branch
from
1efefc9 to
3264e62
Compare
| choices=PACKAGE_TYPES, | ||
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) |
There was a problem hiding this comment.
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More.
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) | ||
| category = models.ForeignKey(PackageCategory, blank=True, null=True, on_delete=models.SET_NULL) |
There was a problem hiding this comment.
Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. More info.
| name = models.CharField(max_length=255) | ||
| version = models.CharField(max_length=255) | ||
| epoch = models.CharField(max_length=255, blank=True, null=True) | ||
| release = models.CharField(max_length=255, blank=True, null=True) | ||
| arch = models.CharField(max_length=255) | ||
| packagetype = models.CharField(max_length=1, blank=True, null=True) | ||
| category = models.CharField(max_length=255, blank=True, null=True) |
There was a problem hiding this comment.
| category = models.CharField(max_length=255, blank=True, null=True) | |
| category = models.CharField(max_length=255, blank=True, default='') |
Similarly, consider replacing null=True with default="" (and blank=True to pass validation checks).
furlongm
force-pushed
the
gentoo-client-support
branch
from
3264e62 to
9493ca8
Compare
| choices=PACKAGE_TYPES, | ||
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) |
There was a problem hiding this comment.
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More details.
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) | ||
| category = models.ForeignKey(PackageCategory, blank=True, null=True, on_delete=models.SET_NULL) |
There was a problem hiding this comment.
Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. Explained here.
| name = models.CharField(max_length=255) | ||
| version = models.CharField(max_length=255) | ||
| epoch = models.CharField(max_length=255, blank=True, null=True) | ||
| release = models.CharField(max_length=255, blank=True, null=True) | ||
| arch = models.CharField(max_length=255) | ||
| packagetype = models.CharField(max_length=1, blank=True, null=True) | ||
| category = models.CharField(max_length=255, blank=True, null=True) |
There was a problem hiding this comment.
| category = models.CharField(max_length=255, blank=True, null=True) | |
| category = models.CharField(max_length=255, blank=True, default='') |
As above, consider replacing null=True with default="" (and blank=True to pass validation checks).
reports/utils.py
Outdated
| with transaction.atomic(): | ||
| repo_arch, created = machine_arches.get_or_create(name='any') | ||
|
|
||
| repo_name = f'Gentoo Linux' |
There was a problem hiding this comment.
| repo_name = f'Gentoo Linux' | |
| repo_name = 'Gentoo Linux' |
f-string is unnecessary here. This can just be a string. Read more.
furlongm
force-pushed
the
gentoo-client-support
branch
from
9493ca8 to
a133181
Compare
There was a problem hiding this comment.
Looks good. Worth considering though. View full project report here.
| choices=PACKAGE_TYPES, | ||
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) |
There was a problem hiding this comment.
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More info.
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) | ||
| category = models.ForeignKey(PackageCategory, blank=True, null=True, on_delete=models.SET_NULL) |
There was a problem hiding this comment.
Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. More info.
| name = models.CharField(max_length=255) | ||
| version = models.CharField(max_length=255) | ||
| epoch = models.CharField(max_length=255, blank=True, null=True) | ||
| release = models.CharField(max_length=255, blank=True, null=True) | ||
| arch = models.CharField(max_length=255) | ||
| packagetype = models.CharField(max_length=1, blank=True, null=True) | ||
| category = models.CharField(max_length=255, blank=True, null=True) |
There was a problem hiding this comment.
| category = models.CharField(max_length=255, blank=True, null=True) | |
| category = models.CharField(max_length=255, blank=True, default='') |
Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).
furlongm
force-pushed
the
gentoo-client-support
branch
from
a133181 to
4a2c475
Compare
| choices=PACKAGE_TYPES, | ||
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) |
There was a problem hiding this comment.
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Explained here.
| blank=True, | ||
| null=True) | ||
| packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True) | ||
| category = models.ForeignKey(PackageCategory, blank=True, null=True, on_delete=models.SET_NULL) |
There was a problem hiding this comment.
Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. More details.
| name = models.CharField(max_length=255) | ||
| version = models.CharField(max_length=255) | ||
| epoch = models.CharField(max_length=255, blank=True, null=True) | ||
| release = models.CharField(max_length=255, blank=True, null=True) | ||
| arch = models.CharField(max_length=255) | ||
| packagetype = models.CharField(max_length=1, blank=True, null=True) | ||
| category = models.CharField(max_length=255, blank=True, null=True) |
There was a problem hiding this comment.
| category = models.CharField(max_length=255, blank=True, null=True) | |
| category = models.CharField(max_length=255, blank=True, default='') |
Again, consider replacing null=True with default="" (and blank=True to pass validation checks).
No description provided.