feat(mobile-app): Add default vcs base_ref parsing for mobile-app subcommand #2706
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
rbro112
changed the title
|
Deferring review until after HackWeek – please ping me if you need a review more urgently |
src/commands/mobile_app/upload.rs
Outdated
| // Try to get the current ref from the VCS if not provided | ||
| // Note: git_repo_head_ref will return an error for detached HEAD states, | ||
| // which .ok() converts to None - this prevents sending "HEAD" as a branch name | ||
| // In that case, the user will need to provide a valid branch name. |
There was a problem hiding this comment.
This comment appears to be outdated, as we are no longer calling .ok() on None here
rbro112
force-pushed
the
ryan/add_default_vcs_base_ref_parsing_for_mobile-app_subcommand
branch
from
f6fa2a9 to
76521d3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
rbro112
force-pushed
the
ryan/add_default_vcs_base_ref_parsing_for_mobile-app_subcommand
branch
from
c4afc9d to
b83b917
Compare
| // Try to get the current ref from the VCS if not provided | ||
| // Note: git_repo_head_ref will return an error for detached HEAD states, | ||
| // which the error handling converts to None - this prevents sending "HEAD" as a branch name | ||
| // In that case, the user will need to provide a valid branch name. |
There was a problem hiding this comment.
[question] Does this imply that the head_ref is required? From what I can tell right now, we don't validate its presence locally, so if it is required by the server, we would likely get a 4xx response when attempting the upload. If that is the case, we should likely return an error here, rather than swallow the error.
There was a problem hiding this comment.
It is not required, it is optional on the backend so no response failure if not provided.
rbro112
deleted the
ryan/add_default_vcs_base_ref_parsing_for_mobile-app_subcommand
branch
Adds default handling of
head_ref(branch name) to themobile-app uploadsubcommand. This will leverage thehead.shorthand()value if no value is provided by the user.Also handles the detached HEAD state with some logging that will require the user to specify using the command arguments.