New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DRAFT] Creating a separate branch for changes related to new APIs #845
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # pyproject.toml
despite of `mlcube-dir` param in `mlcube.yaml`, place of model file is hardcoded to `/embeddedd_model/` while building an image via `gandlf deploy`
Hi folks!
|
…dardize_cli_options
Trigger tests for pull requests to all branches
…dardize_cli_options
Update PyTorch dependency
…dardize_cli_options
…ardize_cli_options Updated CLI options for new API
Apologies, somehow I missed this comment, @VukW!
This should be done.
How difficult would it be to add new tests? To note that this branch is supposed to stay as a separate branch (and not get merged into master) for at least a few months. |
@sarthakpati 😱Why so? It's not a big problem to add a new test, but it may be disturbing to support two branches (this and main) simultaneously - especially if we'd need to fix / improve anything in entrypoints scripts (that's highly probable on the range of a few months) |
I agree (and the idea to delay the merge is more of a strategic endeavor than anything else). Do you think it makes sense to move the master the newer API and keep a "legacy" branch? I am honestly agnostic to either, just that we need to delay the actual release of the new API branch for a few months. |
Can you plz describe the strategic reason a bit wider as I don't get it quite well? I mean, depending on why exactly we want to wait without merging, I am imaginating different possible solutions. But as this PR is not breaking old behavior, in most of cases merging it looks better for me rather then supporting two branches. Different options I do see: a. Merge as-is. We do not break Say, we have the following confusions:
So, for me it seems like merging the new CLI or declining it totally is always better (from the terms of supportability) then keeping two branches. And if keep two branches - it might be possible, but for some finite period of time until we decide what to do further. |
@VukW: Wow, that was elaborate 😄 Anyway, the reason behind having a separate branch was to delay the actual "release" (i.e., into a tag) of the new API for a few months. But your assessment is completely spot-on. Considering we are a tight knit group of contributors, I vote for option |
@sarthakpati 👍 So, we are not pushing new release / new tag / new wheel package for now, but we kindly ask for all new PRs to be based on this branch instead of master, right? In this way it's easier for us to merge any new features / fixes rather then merge them to master & solve conflicts after |
Yes, precisely! But I would suggest let's wait for @sylwiamm to wrap up the logging work and then we proceed with this. Thoughts? |
Fixes #ISSUE_NUMBER
Proposed Changes
Checklist
CONTRIBUTING
guide has been followed.typing
is used to provide type hints, including and not limited to usingOptional
if a variable has a pre-defined value).pip install
step is needed for PR to be functional), please ensure it is reflected in all the files that control the CI, namely: python-test.yml, and all docker files [1,2,3].