feat(vcs): add read-only jj repository support#31
feat(vcs): add read-only jj repository support#31frittlechasm wants to merge 4 commits intomabd-dev:feat/jjSupportfrom
Conversation
Skip rendering the remote name in parentheses when: - There is only one remote and it is named "origin" (the common default) - The remote name is empty (no remote configured) Closes mabd-dev#20 Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Introduce a VCS provider layer and generalize repo discovery so reposcan can detect and scan both git and jj repositories.
mabd-dev
left a comment
mabd-dev
left a comment
There was a problem hiding this comment.
thank you for the contribution. Overall the code look clean and well made
| TypeJJ Type = "jj" | ||
| ) | ||
|
|
||
| type RepoPath struct { |
There was a problem hiding this comment.
Rename to RepoInfo or something similar that makes more sense
There was a problem hiding this comment.
Makes sense will get it done ..
| VCSType string `json:"vcsType"` | ||
| Branch string `json:"branch"` | ||
| UncommitedFiles []string `json:"uncommitedFiles"` | ||
| OutgoingCommits []string `json:"outgoingCommits"` |
There was a problem hiding this comment.
why this exists here? i don't like the idea that only support jj. same thing if we have a variable here that is only supported by git.
Maybe this struct can be remodeled to handle both jj and git repo. For now, it's this is fine. Will look into it later
There was a problem hiding this comment.
In jj, a commit is the unit of work so more than looking at uncommitted files , being able to see the number of outgoing commits makes more sense.
I can either remove OutgoingCommits entirely for now and use RemoteStatus.Ahead which should give us the count of number of commits which are ahead. OutgoingCommits would have actually shown what those commits are.
Another approach is to add GIT support for Outgoing Commits. This way there will be no variables that are vcs specific.
Let me know which approach you think is best here.
There was a problem hiding this comment.
It might more sense to add OutgoingCommits to RemoteStatus instead of Repostate as conceptually it belongs near sync state.
There was a problem hiding this comment.
Making git support outgoing commits make sense. And yes, I do agree that OutgoingCommits should be in RemoteStatus not RepoState
| } | ||
|
|
||
| func (p *Provider) CheckRepoState(path string) (report.RepoState, []string) { | ||
| state, warnings := gitx.CheckRepoState(path) |
There was a problem hiding this comment.
hmm, should we move all gitx package files to vcs/git package?
I guess this is the right thing to do, since they will only be used in here
There was a problem hiding this comment.
Makes sense I will move the gitx package files to internal/vcs/git ..
I will keep the internal/render/tui as is for now as we have no jj actions.
Let me know if that's okay.
If not I will create interfaces similar to internal/vcs/provider.go for the various actions.
There was a problem hiding this comment.
yes, keep internal/render/tui as is for now
| Include bool | ||
| } | ||
|
|
||
| func GetRepoStatesConcurrent( |
There was a problem hiding this comment.
This makes the git version not needed. Delete internal/gitx/gitxConcurrent.go
There was a problem hiding this comment.
Done will delete it.
3 participants
Adds read-only support for scanning jj repositories alongside Git repositories.
Changes include: