Thanks for the dist-git patch [1]!

There is an issue with naming, what does "dist-git" mean?  Is that copr's
dist-git?  Is that Fedora's dist-git?  Is that Red Hat Internal dist-git?

Can each package be built from different dist-git instance?  If not, why?

+class DistGitProvider(SrpmBuilderProvider):
+    def get_srpm(self):
+        cmd = ['git', 'clone', self.task.distgit_clone_url, self.tmp_dest]
+        output, error = VM.run(cmd, dst_dir=self.tmp_dest, 
name=self.task.task_id)
+        log.info(output)
+
+        if self.task.distgit_branch:
+            cmd = ['fedpkg', '--dist', self.task.distgit_branch, 'srpm']
+        else:
+            cmd = ['fedpkg', 'srpm']

Please consider naming this Provider like FedoraDistGit, and rename
everything API related into Fedora namespace, because this is not generic
"dist-git".  If we have it "renamed", this would be fine contribution ...
but otherwise we waste namespace in api without a way back, and forever --
DON'T RELEASE YET.

Internally (RHEL) we also have dist-git, and it would be really nice to have
this configurable so we can reuse this, or add RHELDistGit, too..

+        output, error = VM.run(cmd, dst_dir=self.tmp_dest, 
name=self.task.task_id, cwd=self.tmp_dest)

Please consider moving all this expensive tasks onto builder.  That's the only
_secure_ way to add "SRPM providers" and it is the only scalable way (koji does
this right, we can possibly copy code from there - or at least study).  Also,
VM != virtualization in this case, which is confusing.

+        log.info(output)
+        self.copy()
...

Seriously, this is basically good code, but start doing reviews so people
involved can discuss design issues.

[1] https://github.com/fedora-copr/copr/commit/308daed9235cb

Pavel

_______________________________________________
copr-devel mailing list -- copr-devel@lists.fedorahosted.org
To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org

Reply via email to