juergbi commented on code in PR #111:
URL: 
https://github.com/apache/buildstream-plugins/pull/111#discussion_r4104455862


##########
src/buildstream_plugins/sources/git.py:
##########
@@ -1097,6 +1101,8 @@ def _guess_version(self):
         return (commit_sha, version_guess, commits)
 
     def collect_source_info(self):
+        if self.mirror.ref is None:
+            return []

Review Comment:
   Sorry for the review delay. The `_guess_version()` improvement looks good to 
me. However, I'm not convinced we actually want/need the early return in 
`collect_source_info()`.
   
   If we want to prevent collecting info of unresolved sources, I think we 
should consider this in BuildStream core, not require plugins to handle that 
case. The `_guess_version()` improvement is sufficient to fix the crash, as far 
as I can tell.
   
   Do you have a good argument for this early return or are you ok with 
dropping that part?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to