Skip to content
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

Extract concepts of local and remote git repos #4900

Closed
wants to merge 6 commits into from
Closed

Conversation

mislav
Copy link
Contributor

@mislav mislav commented Dec 13, 2021

The browse command queries the GitClient object it received for information like "LastCommit" and "PathPrefix". Depending on the --repo flag, the repository queried will be either the local git repository (normal case) or the remote git repository via the GitHub API (in case of the --repo override).

This is a followup to #4845 by @bchadwic with the goal of extracting and exposing concepts like a "local git repo" and "remote git repo" behind a unified interface. This will be useful for all commands that support the --repo override and need access to git information.

Design goals:

  • Local and remote git repositories are queried using an identical interface
  • The remote git repository implementation should raise an ErrNotImplemented for operations that are unfeasible to implement via the API
  • Each query method now supports passing a context.Context object to support cancellation at a lower level.

TODO:

  • Allow querying the local git repo from a path other than the current directory - this is meant to replace various git.*WithPath() methods
  • Iron out how remoterepo.Client gains access to *http.Client and ghrepo.Interface - ideally it wouldn't rely on passing funcs around anymore
  • Mark legacy git.* methods as deprecated - all future queries should go through an instantiated client

This introduces a concept of a Git object that is asked for git
operations (only Push for now) from the context of a command. Instead of
stubbing out the `git push` command, the call to `Push()` itself is
stubbed via mock object generated by mockery.

Additionally, this introduces tests that exercise the logic of shelling
out to `git push`, which is now rewritten to ensure synchronous I/O.
The `browse` command queries the GitClient object it received for
information like "LastCommit" and "PathPrefix". Depending on the
`--repo` flag, the repository queried will be either the local git
repository (normal case) or the remote git repository via the GitHub API
(in case of the repo override).

Each git query method now supports passing a `context.Context` object to
support cancellation at a lower level.
@mislav mislav self-assigned this Dec 13, 2021
@bchadwic
Copy link
Contributor

bchadwic commented Dec 13, 2021

Hey thanks for moving forward with the prototype, I'm looking forward to seeing how this turns out.

Just in case you didn't see it, I suggested that the LastCommit error should be changed in the last pr.

#4845 (comment)

Maybe this can be added into this pr for a better experience, especially now that the last commit can come from two different sources.

@mislav
Copy link
Contributor Author

mislav commented Dec 14, 2021

Just in case you didn't see it, I suggested that the LastCommit error should be changed in the last pr.

Ah, thanks for reminding me. I missed that comment. I will make sure to address it in this PR 👍

@vilmibm
Copy link
Contributor

vilmibm commented Jan 5, 2022

i like this idea~ thanks for working on it

@mislav
Copy link
Contributor Author

mislav commented Oct 24, 2022

Superseded by #6354

@mislav mislav closed this Oct 24, 2022
@mislav mislav deleted the git-client branch October 24, 2022 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants