gtristan commented on code in PR #1907:
URL: https://github.com/apache/buildstream/pull/1907#discussion_r1549687405
##########
src/buildstream/_frontend/app.py:
##########
@@ -909,13 +905,10 @@ def _assert_element_path(self, element_path):
# min_version (int): The user selected minimum BuildStream version
# element_path (str): The user selected element path
#
- def _init_project_interactive(self, project_name, min_version=None,
element_path="elements"):
+ def _init_project_interactive(self, project_name, min_version,
element_path):
bst_major, bst_minor = utils._get_bst_api_version()
- if min_version is None:
Review Comment:
Ok I understand now and I've read through the other usages of the version
numbers (checking if the selected versions are "known" by installed
buildstream).
What looks ambiguous to me, is the possibility for `min_version` to be
`None`, which the function allows but never happens because it is always
invoked with a hardcoded value.
Let's at least:
* Update `App.init_project()` docs comment for `min_version` and
`element_path`, removing the notes about what the defaults are when you pass
`None` here
* Same for `App.init_project_interactive()`
Ideally we should be moving `App` to use type annotations also, but we can
do that separately...
--
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]
