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

Graphql, Web: pull from and push to remotes #315

Merged
merged 12 commits into from
Dec 6, 2024
Merged

Conversation

liuliu-dev
Copy link
Contributor

@liuliu-dev liuliu-dev commented Dec 5, 2024

actions dropdown:
Screenshot 2024-12-05 at 3 45 27 PM

push
Screenshot 2024-12-05 at 3 46 22 PM

pull
Screenshot 2024-12-05 at 3 46 30 PM

@liuliu-dev liuliu-dev requested a review from tbantle22 December 5, 2024 21:19
Copy link
Collaborator

@tbantle22 tbantle22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. A few copy edits and graphql types comments

Copy link
Collaborator

@tbantle22 tbantle22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more small things but otherwise lgtm

@@ -291,6 +291,17 @@ type RemoteList {
list: [Remote!]!
}

type PullRes {
fastForward: Boolean!
conflicts: Float!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need _type => Int in this Field in the model


export function fromPullRes(r: RawRow): PullRes {
return {
fastForward: r.fast_forward !== "0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should both be r.fast_forward === "1". In case some weird int gets in there you want it to be false in all circumstances except for 1


export function fromPushRes(r: RawRow): PushRes {
return {
status: r.status === "0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since status is kinda unclear you could make this field success if you want

@liuliu-dev liuliu-dev merged commit 290785f into main Dec 6, 2024
2 checks passed
@liuliu-dev liuliu-dev deleted the liuliu/pull-and-push branch December 6, 2024 21:09
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.

2 participants