#72 Patch over ActivityPub

Open
opened 3 months ago by fr33domlover · 5 comments

I'd like to make a simple initial representation of an ActivityPub version of sending-patches-by-email. Ignore the whole Merge Request story, just plain sending a patch authored locally by the client. How would we represent it?

PROPOSAL:

Have a ForgeFed type Patch. Properties:

  • attributedTo: Who wrote it
  • mediaType: A VCS-specific value, e.g. Darcs uses something like application/x-darcs-patch iirc, it can be found in the Darcs source code. Idk what Git uses, but it's always possible to use a media type for standard diff, or just text/plain or some custom generic value we define in ForgeFed
  • content: The actual patch, in the native VCS specific format

An alternative is to define a universal patch format that supports all the VCSs, but Idk if there's a good reason to make that effort. As long as there's no need, let's just use whatever format VCSs natively use.

To send the patch to a project, you make an Offer activity whose object is the patch.

But to whom exactly do you send the offer, to which actor? Should we always send it to the Repository? Conceptually, you send your patches to people to review and possibly apply them. A minimal Repository just manages the settings and events of the VCS repo.

PROPOSAL: There's a trivial way to make it flexible: Like the ticketsTrackedBy property for submitting tickets, we can have a sendPatchesTo property. Each Repository can set this property. If it handles its own patches, it sends that property to its own ID. If a specific user handles the patches, then it's the ID of that user. And if it's some other thing such as Project or TicketTracker or whatever, then the ID of that thing is specified.

What does it mean that a Patch is accepted?

When a Ticket is accepted, it means it got added as an open work item. But for a patch, it's not clear: Does it mean the patch is waiting for review, or does it mean the patch got reviewed and approved and waiting to be applied? Or does it mean the patch got applied?

I don't want to hurry to decide on this, because it may be a good idea to handle this in the same way we'll handle Merge Requests, for consistency. But let's see what the options are.

One idea is that Accepting a patch means it got listed as a work item. In that case, we may be able to avoid representing events like "patch got review" or "patch got approved" or "patch got applied", because from now on the work is managed via the work item, i.e. the Ticket that got created. But there's a problem: It requires that patches are always turned into work items. What if a person just has a personal to-do list outside of ForgeFed, and there's no ticket tracking? In that case we can still send an Accept, simply omitting the ticket ID since there's no ticket, but now how do we report events like patch review status and patch-being-applied?

I'm also wondering whether all those events even need dedicated activities. I suppose there are 2 reasons to have those: One is functionality, another is display and localization. A simple Create Note is limited to text, while a dedicated type can be more wisely used by client UI.

I haven't done research on this yet, but here are the basics steps in the lifetime of a patch-by-email:

  • It gets sent
  • There's discussion about it
  • There are new versions with fixes
  • Eventually someone says, ok let's apply it
  • The patch is applied and pushed into the repo

PROPOSAL:

  • Patch is sent: Offer { Patch }
  • Discussion about patch: Create { Note }
  • New versions and fixes: Hmmm I'm unsure, we can do Update{Patch} or Update{Offer{Patch}} or idk what else, let's discuss this; even without this the workflow of patch-is-sent-and-applied is possible
  • Someone says ok let's apply it: We could have review related activities but for now it can just be a plain Create { Note }
  • Patch is applied and pushed to repo: As much as I'd like to have a cool Apply activity, none of the previous steps Accepted the patch, so... Accepting the Offer will mean applying the patch

But again we have a problem: If the patch gets a work item attached, how will the patch author be notified? A Create{Ticket} could be sent, but now there's slight weirdness where Offer{Ticket} doesn't require a Create while Offer{Patch} does. Unless we require Offer{Ticket} to result with a Create. Slightly more verbose than plain Accept but possible.

Another option: if the Accept has a result whose type is Ticket, it means a ticket got opened, otherwise it means the patch got applied. Hmm nope, this isn't good. Because then, if we already Accepted when we opened the ticket, how do we report applying the patch? Another Accept on the same Offer? Nope, not good.

Sorry for the chaos of words, everyone. Just recording my thoughts.

QUESTION: Is it reasonable to Update activities after you sent them? If so, why do Created objects get a direct Update while objects embedded in other activities would get an Update for the activity?

IDEA: First you publish a Patch, e.g. using a Create activity. Then, you send an Offer linking to the patch. The discussion topic is the patch, not the offer. So people comment on the Patch by sending Create{Note} with context (and inReplyTo) pointing to the patch, just like in ticket discussion. If a work item happens to be opened for the patch, then that's a separate Create{Ticket}. When the patch is applied, an Accept is sent.

The problem with this idea is that the canonical patch is hosted by the author and not by the project. An alternative way is to switch to hosting it on the project side, but then the author needs to be granted access to Update it. That can work much like with Tickets, where the ticket author would probably have access to update the ticket after opening it, which is usually possible in forges (e.g. I can update this issue after I open it).

Let's see this alternative scenario.

PROPOSAL:

  • Patch gets sent: Offer{Patch}, followed by an Accept whose result points to the patch, or possibly to a Ticket but let's put the ticket idea aside for now
  • There's discussion about it: Create{Note} whose context is the patch hosted on the project side
  • There are new versions with fixes: Update{Patch}
  • Eventually someone says, ok let's apply it: Create{Note}
  • The patch is applied and pushed into the repo: A new dedicated Apply activity we define in ForgeFed

Lots of thoughts. Puts a doubt on everything lol. Let the discussion begin ^_^

I'd like to make a simple initial representation of an ActivityPub version of sending-patches-by-email. Ignore the whole Merge Request story, just plain sending a patch authored locally by the client. How would we represent it? PROPOSAL: Have a ForgeFed type `Patch`. Properties: - `attributedTo`: Who wrote it - `mediaType`: A VCS-specific value, e.g. Darcs uses something like `application/x-darcs-patch` iirc, it can be found in the Darcs source code. Idk what Git uses, but it's always possible to use a media type for standard diff, or just `text/plain` or some custom generic value we define in ForgeFed - `content`: The actual patch, in the native VCS specific format An alternative is to define a universal patch format that supports all the VCSs, but Idk if there's a good reason to make that effort. As long as there's no need, let's just use whatever format VCSs natively use. To send the patch to a project, you make an `Offer` activity whose `object` is the patch. But to whom exactly do you send the offer, to which actor? Should we always send it to the `Repository`? Conceptually, you send your patches to **people** to review and possibly apply them. A minimal `Repository` just manages the settings and events of the VCS repo. PROPOSAL: There's a trivial way to make it flexible: Like the `ticketsTrackedBy` property for submitting tickets, we can have a `sendPatchesTo` property. Each `Repository` can set this property. If it handles its own patches, it sends that property to its own ID. If a specific user handles the patches, then it's the ID of that user. And if it's some other thing such as `Project` or `TicketTracker` or whatever, then the ID of that thing is specified. What does it mean that a `Patch` is accepted? When a `Ticket` is accepted, it means it got added as an open work item. But for a patch, it's not clear: Does it mean the patch is waiting for review, or does it mean the patch got reviewed and approved and waiting to be applied? Or does it mean the patch got applied? I don't want to hurry to decide on this, because it may be a good idea to handle this in the same way we'll handle Merge Requests, for consistency. But let's see what the options are. One idea is that `Accept`ing a patch means it got listed as a work item. In that case, we may be able to avoid representing events like "patch got review" or "patch got approved" or "patch got applied", because from now on the work is managed via the work item, i.e. the `Ticket` that got created. But there's a problem: It requires that patches are *always* turned into work items. What if a person just has a personal to-do list outside of ForgeFed, and there's no ticket tracking? In that case we can still send an `Accept`, simply omitting the ticket ID since there's no ticket, but now how do we report events like patch review status and patch-being-applied? I'm also wondering whether all those events even need dedicated activities. I suppose there are 2 reasons to have those: One is functionality, another is display and localization. A simple `Create Note` is limited to text, while a dedicated type can be more wisely used by client UI. I haven't done research on this yet, but here are the basics steps in the lifetime of a patch-by-email: - It gets sent - There's discussion about it - There are new versions with fixes - Eventually someone says, ok let's apply it - The patch is applied and pushed into the repo PROPOSAL: - Patch is sent: `Offer { Patch }` - Discussion about patch: `Create { Note }` - New versions and fixes: Hmmm I'm unsure, we can do `Update{Patch}` or `Update{Offer{Patch}}` or idk what else, let's discuss this; even without this the workflow of patch-is-sent-and-applied is possible - Someone says ok let's apply it: We could have review related activities but for now it can just be a plain `Create { Note }` - Patch is applied and pushed to repo: As much as I'd like to have a cool `Apply` activity, none of the previous steps `Accept`ed the patch, so... `Accept`ing the Offer will mean applying the patch But again we have a problem: If the patch gets a work item attached, how will the patch author be notified? A `Create{Ticket}` could be sent, but now there's slight weirdness where `Offer{Ticket}` doesn't require a `Create` while `Offer{Patch}` does. Unless we require `Offer{Ticket}` to result with a `Create`. Slightly more verbose than plain `Accept` but possible. Another option: if the `Accept` has a `result` whose type is `Ticket`, it means a ticket got opened, otherwise it means the patch got applied. Hmm nope, this isn't good. Because then, if we already `Accept`ed when we opened the ticket, how do we report applying the patch? Another `Accept` on the same `Offer`? Nope, not good. Sorry for the chaos of words, everyone. Just recording my thoughts. QUESTION: Is it reasonable to `Update` activities after you sent them? If so, why do `Create`d objects get a direct `Update` while objects embedded in other activities would get an `Update` for the activity? IDEA: First you publish a `Patch`, e.g. using a `Create` activity. Then, you send an `Offer` linking to the patch. The discussion topic is *the patch, not the offer*. So people comment on the `Patch` by sending `Create{Note}` with `context` (and `inReplyTo`) pointing to the patch, just like in ticket discussion. If a work item happens to be opened for the patch, then that's a separate `Create{Ticket}`. When the patch is applied, an `Accept` is sent. The problem with this idea is that the canonical patch is hosted by the author and not by the project. An alternative way is to switch to hosting it on the project side, but then the author needs to be granted access to `Update` it. That can work much like with `Ticket`s, where the ticket author would probably have access to update the ticket after opening it, which is usually possible in forges (e.g. I can update this issue after I open it). Let's see this alternative scenario. PROPOSAL: - Patch gets sent: `Offer{Patch}`, followed by an `Accept` whose `result` points to the patch, or possibly to a `Ticket` but let's put the ticket idea aside for now - There's discussion about it: `Create{Note}` whose `context` is the patch hosted on the project side - There are new versions with fixes: `Update{Patch}` - Eventually someone says, ok let's apply it: `Create{Note}` - The patch is applied and pushed into the repo: A new dedicated `Apply` activity we define in ForgeFed Lots of thoughts. Puts a doubt on everything lol. Let the discussion begin ^_^
fr33domlover commented 3 months ago
Collaborator

Problem with the stuff above: How to separate comments on different versions of the patch? It would be possible if the comment topic is the Offer/Update and not the patch itself. But hmmm those are canonically hosted by the user, not by the project. Hmmm another option is you send a whole new Offer for each update, and a new patch is created remotely, and comments go there. Another possibility is that the distinction is only done by chronologically listing events in UI display. It's much like in Merge Requests you can force-push, deleting some diffs that already got comments on them.

Problem with the stuff above: How to separate comments on different versions of the patch? It would be possible if the comment topic is the `Offer`/`Update` and not the patch itself. But hmmm those are canonically hosted by the user, not by the project. Hmmm another option is you send a whole new `Offer` for each update, and a new patch is created remotely, and comments go there. Another possibility is that the distinction is only done by chronologically listing events in UI display. It's much like in Merge Requests you can force-push, deleting some diffs that already got comments on them.
fr33domlover commented 2 months ago
Collaborator

The decision on hosting tickets changes the situation, now we have both the Create flow and the Offer flow. I'm starting below a new proposal. First in a verbose form with all my thoughts, and later I'll add a short practical summary.

Thoughts

A patch is essentially a work item, "review this code". A patch can have a priority, e.g. it can be a fix for a severe security bug. It can be assigned to a team member to review it. It can get a due date. It can belong to a milestone. It's essentially a ticket with a piece of code attached.

When you propose a patch to a repo, I think you should be able to set the due date and priority and so on. Sure, you could separately send a patch and then open a ticket that mentions it, but it's kind of weird to keep them separate like that. Especially since patches-over-email get to have associated text and can get labels/tags in the email subject. What if submission of a patch is always a submission of a ticket, with a patch object attached?

Suppose for simplicity for a moment, that the patch object is embedded into the same JSON-LD document as the ticket. For example, you'd be sending a Create Ticket in which the Ticket has an attachment of type Patch.

Let's see how patch related events would be represented.

Create flow:

  • Patch gets sent: Create Ticket, followed by an Accept on the Create
  • Discussion about patch: Create Note whose context is the ticket
  • New versions and fixes: Update Ticket
  • Eventually someone says let's apply it: Create Note i.e. just a comment
  • Patch is applied and pushed into repo: Accept on the patch itself, or a dedicated Apply activity

Code review could be Create Note whose context/topic isn't the ticket, but instead a specific piece of code.

As discussed in #85, we need to represent the different patch versions. I suggested to do that using an OrderedCollection of Patch objects. The problem is that if the patches are specified using attachment, it's kind of weird for implementations to understand what sort of thing is being attached. SO another option is to use a dedicated custom property instead of attachment.

Should we use an OrderedCollection of Patch objects (or maybe just their ID URIs!) under attachment or under a custom property?

I opened a forum thread about this, waiting for feedback.

The decision on hosting tickets changes the situation, now we have both the Create flow and the Offer flow. I'm starting below a new proposal. First in a verbose form with all my thoughts, and later I'll add a short practical summary. # Thoughts A patch is essentially a work item, "review this code". A patch can have a priority, e.g. it can be a fix for a severe security bug. It can be assigned to a team member to review it. It can get a due date. It can belong to a milestone. It's essentially a ticket with a piece of code attached. When you propose a patch to a repo, I think you should be able to set the due date and priority and so on. Sure, you could separately send a patch and then open a ticket that mentions it, but it's kind of weird to keep them separate like that. Especially since patches-over-email get to have associated text and can get labels/tags in the email subject. What if submission of a patch is always a submission of a ticket, with a patch object attached? Suppose for simplicity for a moment, that the patch object is embedded into the same JSON-LD document as the ticket. For example, you'd be sending a `Create Ticket` in which the `Ticket` has an `attachment` of type `Patch`. Let's see how patch related events would be represented. Create flow: - Patch gets sent: `Create Ticket`, followed by an `Accept` on the `Create` - Discussion about patch: `Create Note` whose `context` is the ticket - New versions and fixes: `Update Ticket` - Eventually someone says let's apply it: `Create Note` i.e. just a comment - Patch is applied and pushed into repo: `Accept` on the patch itself, or a dedicated `Apply` activity Code review could be `Create Note` whose context/topic isn't the ticket, but instead a specific piece of code. As discussed in #85, we need to represent the different patch versions. I suggested to do that using an `OrderedCollection` of `Patch` objects. The problem is that if the patches are specified using `attachment`, it's kind of weird for implementations to understand what sort of thing is being attached. SO another option is to use a dedicated custom property instead of `attachment`. Should we use an `OrderedCollection` of `Patch` objects (or maybe just their ID URIs!) under `attachment` or under a custom property? I opened a [forum thread](https://socialhub.activitypub.rocks/t/representing-patch-versions/483) about this, waiting for feedback.
fr33domlover commented 2 months ago
Collaborator

Oh, how do we represent the target repo/branch of a patch/MR? I opened #86 for this.

Oh, how do we represent the target repo/branch of a patch/MR? I opened #86 for this.
fr33domlover commented 2 months ago
Collaborator

Another possibility: The attachment of the Ticket would be an Offer object, with target and origin referring to repos/branches, and object being an OrderedCollection of Patches.

Another possibility: The `attachment` of the `Ticket` would be an `Offer` object, with `target` and `origin` referring to repos/branches, and `object` being an `OrderedCollection` of `Patch`es.
fr33domlover commented 2 months ago
Collaborator

Waiting for #88. Once that issue is closed, proceed here.

Waiting for #88. Once that issue is closed, proceed here.
Sign in to join this conversation.
Loading...
Cancel
Save
There is no content yet.