[Patchew-devel] [PATCH] Refactoring of UpdateProjectHeadView.handle()
Paolo Bonzini
pbonzini at redhat.com
Wed May 23 09:19:47 UTC 2018
On 22/05/2018 21:13, Shubham Jain wrote:
> moved and refactored the UpdateProjectHeadView.handle() into method of api.models.Project so that it can be re-used in rest conversion of update-project-head
> ---
> api/models.py | 42 ++++++++++++++++++++++++++++++++++++++++++
> api/views.py | 25 +------------------------
> 2 files changed, 43 insertions(+), 24 deletions(-)
>
> diff --git a/api/models.py b/api/models.py
> index d602cb7..be1e2be 100644
> --- a/api/models.py
> +++ b/api/models.py
> @@ -44,6 +44,33 @@ def load_blob_json(name):
> logging.error('Failed to load blob %s: %s' %(name, e))
> return None
>
> +def get_series_to_be_updated(project_name, message_ids):
> + updated_series = []
> + for msgid in message_ids:
> + if msgid.startswith("<") and msgid.endswith(">"):
> + msgid = msgid[1:-1]
> + p = Project.objects.get(name=project_name)
> + mo = Message.objects.filter(project=p, message_id=msgid,
> + is_merged=False).first()
> + if not mo:
> + continue
> + mo.is_merged = True
> + mo.save()
> + s = mo.get_series_head()
> + if s:
> + updated_series.append(s)
> + return updated_series
>
> +def change_merge_status(series):
> + merged = True
> + for p in series.get_patches():
> + if not p.is_merged:
> + merged = False
> + break
> + if merged:
> + series.is_merged = True
> + series.save()
Both methods are small, so I think it's okay to place the two algorithms
in series_update directly. The advantage is that the global namespace
of api.models remains clean.
As an additional cleanup, the for loop in change_merge_status could be
written
for p in series.get_patches():
if not p.is_merged:
break
else:
series.is_merged = True
series.save()
> class Project(models.Model):
> name = models.CharField(max_length=1024, db_index=True, unique=True,
> help_text="""The name of the project""")
> @@ -171,6 +198,21 @@ class Project(models.Model):
> def get_subprojects(self):
> return Project.objects.filter(parent_project=self)
>
> + def get_project_head(self):
> + return self.get_property("git.head")
> +
> + project_head = property(get_project_head)
> +
> + def series_update(self, message_ids):
> + series_to_be_updated = get_series_to_be_updated(self.name, message_ids)
> + ret = len(series_to_be_updated)
> + for s in series_to_be_updated:
> + change_merge_status(s)
> + return ret
> +
> + def set_project_head(self, new_head):
> + self.set_property("git.head", new_head)
You're not using this method and the corresponding getter, and I think
it's okay to leave it in UpdateProjectHeadView. But if you kept it, you
should use "project_head = property(get_project_head, set_project_head)".
Thanks,
Paolo
> class ProjectProperty(models.Model):
> project = models.ForeignKey('Project', on_delete=models.CASCADE)
> name = models.CharField(max_length=1024, db_index=True)
> diff --git a/api/views.py b/api/views.py
> index c27cd5a..4cb10c1 100644
> --- a/api/views.py
> +++ b/api/views.py
> @@ -123,30 +123,7 @@ class UpdateProjectHeadView(APILoginRequiredView):
> old_head_0 = po.get_property("git.head")
> if old_head_0 and old_head_0 != old_head:
> raise Exception("wrong old head")
> - ret = 0
> - updated_series = []
> - for msgid in message_ids:
> - if msgid.startswith("<") and msgid.endswith(">"):
> - msgid = msgid[1:-1]
> - mo = Message.objects.filter(project=po, message_id=msgid,
> - is_merged=False).first()
> - if not mo:
> - continue
> - ret += 1
> - mo.is_merged = True
> - mo.save()
> - s = mo.get_series_head()
> - if s:
> - updated_series.append(s)
> - for s in updated_series:
> - merged = True
> - for p in s.get_patches():
> - if not p.is_merged:
> - merged = False
> - break
> - if merged:
> - s.is_merged = True
> - s.save()
> + ret = po.series_update(message_ids)
> po.set_property("git.head", new_head)
> return ret
>
>
More information about the Patchew-devel
mailing list