modularity support #400
modularity support #400
Conversation
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
furlongm
force-pushed
the
modularity
branch
3 times, most recently
from
a759423 to
cb34c3f
Compare
furlongm
force-pushed
the
modularity
branch
5 times, most recently
from
bb2bb27 to
46a397b
Compare
![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.
There was a problem hiding this comment.
Looks good. Worth considering though. View full project report here.
| except DatabaseError as e: | ||
| error_message.send(sender=None, text=e) | ||
| modules.add(module) | ||
| for package in module.packages.all(): |
There was a problem hiding this comment.
reportUnboundVariable: "module" is possibly unbound
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| modules.add(module) | ||
| for package in module.packages.all(): | ||
| if package.id not in package_ids: | ||
| module.packages.remove(package) |
There was a problem hiding this comment.
reportUnboundVariable: "module" is possibly unbound
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| stream = models.CharField(unique=True, max_length=255) | ||
| version = models.CharField(max_length=255) | ||
| context = models.CharField(unique=True, max_length=255) | ||
| arch = models.ForeignKey(PackageArchitecture, on_delete=models.CASCADE) |
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.
| version = models.CharField(max_length=255) | ||
| context = models.CharField(unique=True, max_length=255) | ||
| arch = models.ForeignKey(PackageArchitecture, on_delete=models.CASCADE) | ||
| repo = models.ForeignKey(Repository, on_delete=models.CASCADE) |
There was a problem hiding this comment.
Again, with an explicit related_name would be better.
| @@ -45,6 +45,7 @@ class Report(models.Model): | |||
| sec_updates = models.TextField(null=True, blank=True) | |||
| bug_updates = models.TextField(null=True, blank=True) | |||
| repos = models.TextField(null=True, blank=True) | |||
| modules = models.TextField(null=True, blank=True) | |||
There was a problem hiding this comment.
| modules = models.TextField(null=True, blank=True) | |
| modules = models.TextField(default='', blank=True) |
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.
| stream = models.CharField(unique=True, max_length=255) | ||
| version = models.CharField(max_length=255) | ||
| context = models.CharField(unique=True, max_length=255) | ||
| arch = models.ForeignKey(PackageArchitecture, on_delete=models.CASCADE) |
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.
| version = models.CharField(max_length=255) | ||
| context = models.CharField(unique=True, max_length=255) | ||
| arch = models.ForeignKey(PackageArchitecture, on_delete=models.CASCADE) | ||
| repo = models.ForeignKey(Repository, on_delete=models.CASCADE) |
There was a problem hiding this comment.
Likewise, with an explicit related_name would be better.
| @@ -45,6 +45,7 @@ class Report(models.Model): | |||
| sec_updates = models.TextField(null=True, blank=True) | |||
| bug_updates = models.TextField(null=True, blank=True) | |||
| repos = models.TextField(null=True, blank=True) | |||
| modules = models.TextField(null=True, blank=True) | |||
There was a problem hiding this comment.
| modules = models.TextField(null=True, blank=True) | |
| modules = models.TextField(default='', blank=True) |
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.
No description provided.