radicle-dev / radicle-client-services

Services backing the Radicle client interfaces.
Apache License 2.0
31 stars 13 forks source link

Use `radicle_surf` `Commit` struct instead of `radicle_source` #196

Closed sebastinez closed 1 year ago

sebastinez commented 2 years ago

This allows us to query parents of a commit and since radicle_source will eventually be deprecated, it prepares for it

Signed-off-by: Sebastian Martinez me@sebastinez.dev

sebastinez commented 2 years ago

CI errors are due to this PR relying on the implementation of https://github.com/radicle-dev/radicle-surf/pull/208 in radicle-surf And since radicle_surf will eventually be migrated to radicle-git would have to be revisited

UPDATE: Related PR is now located at https://github.com/radicle-dev/radicle-git/pull/4

FintanH commented 1 year ago

Is https://github.com/radicle-dev/radicle-git/pull/10 compatible with this change, btw? They're kind of fighting the same battles :)

sebastinez commented 1 year ago

Is radicle-dev/radicle-git#10 compatible with this change, btw? They're kind of fighting the same battles :)

@FintanH I just converted this PR to draft state, since they are many moving parts currently and want the dust to settle a bit 😅

FintanH commented 1 year ago

@FintanH I just converted this PR to draft state, since they are many moving parts currently and want the dust to settle a bit sweat_smile

Would be good to get your eyes on those changes as we make them :)

cloudhead commented 1 year ago

Does this require any changes on the frontend?

sebastinez commented 1 year ago

Does this require any changes on the frontend?

Sorry for the delay was down in router land 😅. Yeah we would need to merge https://github.com/radicle-dev/radicle-interface/pull/396

sebastinez commented 1 year ago

For review a diff on how the commit header output would change

diff --git a/1.json b/2.json
index 3c56fcd..bee84f2 100644
--- a/1.json
+++ b/2.json
@@ -1,16 +1,21 @@
 {
   "header": {
-    "sha1": "47596419a5fcc9c360d5fee90e2fddbe733e1386",
+    "id": "47596419a5fcc9c360d5fee90e2fddbe733e1386",
     "author": {
       "name": "ray.wu",
-      "email": "wgq.quan@gmail.com"
+      "email": "wgq.quan@gmail.com",
+      "time": 1608263931
     },
     "summary": "optimize",
     "description": "",
+    "message": "optimize\n",
     "committer": {
       "name": "ray.wu",
-      "email": "wgq.quan@gmail.com"
+      "email": "wgq.quan@gmail.com",
+      "time": 1608263931
     },
-    "committerTime": 1608263931
+    "parents": [
+      "e10d0070001b3c40e38437bf58329473f0674221"
+    ]
   }
 }
cloudhead commented 1 year ago

Interesting, so the message contains the summary in it. But then what is description? Seems like there's one field too many there.

sebastinez commented 1 year ago

Interesting, so the message contains the summary in it. But then what is description? Seems like there's one field too many there.

Yeah message is part of the surf::Commit while description is a getter that strips the summary. Don't think the message part will come in much handy, would be a change to radicle-git to remove this. Maybe at one point we want to show a raw format of a commit, and then message could come in handy 🤔

sebastinez commented 1 year ago

Hmm ok I took this morning to read over all the design intentions of the radicle-surf redesign. I'm thinking we should maybe close this PR and take another intent at this, once the redesign is finished, since it's not something urgent. wdyt @cloudhead?

cloudhead commented 1 year ago

Yeah sounds good.