Protocols command consumes default branch from env #263
Conversation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## stable #263 +/- ##
==========================================
- Coverage 70.37% 70.35% -0.02%
==========================================
Files 82 82
Lines 7503 7498 -5
Branches 1391 1389 -2
==========================================
- Hits 5280 5275 -5
Misses 1849 1849
Partials 374 374
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| debug: bool = False, | ||
| _: str = CONFIG_PARAM, | ||
| branch: str = typer.Option("main", help="Branch on which to run the script."), | ||
| branch: str = typer.Option("main", help="Branch on which to run the script."), # TODO: Replace main by None |
There was a problem hiding this comment.
I'm not understanding the change here, should it be setting the default to None?
There was a problem hiding this comment.
In my opinion throughout infrahubctl the branch parameter should be an optional string defaulting to None and if not specified we should either not specify the branch or use the default branch provided by the SDK which will default to "main", alternatively something else if the SDK is configured that way with some environment variable.
There was a problem hiding this comment.
Yep with the current setup you can't use environment variable INFRAHUB_DEFAULT_BRANCH.
➜ export INFRAHUB_DEFAULT_BRANCH="test1"
➜ infrahubctl schema load some.yml
The schema in Infrahub was is already up to date, no changes were required
1 schema processed in 0.138 seconds.=> This loads schema to main branch
There was a problem hiding this comment.
Here is the issue to fix all remaining commands #264 I guess it could be breaking someone's workflow ...
There was a problem hiding this comment.
It might, although hopefully it wouldn't be that impactful as any changes would be done against a non main branch. But worth including in a changelog to indicate this updated behaviour. Not sure if we need all those todo comments when we have #264, but they might be easier to find I guess.
* Get default branch from env * Add comment on all hardcoded main branch to be fixed later * Add release note
3 participants
protocolscommand properly gets default branch setting from environment variableresolves: #104