-
Notifications
You must be signed in to change notification settings - Fork 110
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
RSDK-5608 - CLI commands for managing cloud builds #3298
Merged
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
b5baca8
revertme: target local api
abe-winter 4e169ce
stub to trigger StartBuild rpc
abe-winter 01093ea
rm commented code
abe-winter 5569f20
default build info / arch from manifest
abe-winter 72a5209
include moduleId in 'build start' command
abe-winter 57d4236
RSDK-5607 - Cli Viam module build local (#3231)
zaporter-work 2b95437
lint
zaporter-work d9eef89
build list
zaporter-work 296ad30
merge conflicts
zaporter-work 47e7855
include repo + ref in startBuild
abe-winter a04319c
fix bugs in formatting
zaporter-work f9ee988
attempt simplification, removal of job id
zaporter-work 1e988aa
compilation fixes
zaporter-work f96cac4
logs + list improvements
zaporter-work 3c03147
added versions. viam module build list mostly works now
zaporter-work 286370d
cleanup list
zaporter-work c9fdc43
Merge remote-tracking branch 'upstream/main' into cloud-build
zaporter-work 7364fc2
bump api
zaporter-work a8309fc
lint + readd directive
zaporter-work 9df086b
logs + list are fully functional but need tests
zaporter-work c03f258
polish
zaporter-work a49ca92
remove local replace
zaporter-work f1d120d
pr comments
zaporter-work 4f215e7
revert to recv() after becoming confident that the ctx is respected
zaporter-work e56f148
remove unused import
abe-winter 2316031
add some basic unit tests
zaporter-work 4ee828d
forgot to add this file last night
zaporter-work File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to look this up myself but couldn't figure it out. Should
DefaultText
always come with aValue
? Is it thatDefaultText
is a suggested value where asValue
is an actual default?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, this surprising behavior was fixed for the other cli commands in #3162
My understanding is that:
DefaultText
=> shows in help msg, does not actually change the default value when you try to read the flagValue
=> shows in help msg + actual default valueThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup thanks for confirming what I found. I feel like
DefaultText
isn't appropriate then. The "default" is that you don't specify the flag at all, otherwise you specify it and it should be a int.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, as we discussed keep it as-is.