Page MenuHomePhabricator

Improve documentation for differential.setdiffproperty
Closed, WontfixPublic

Description

The documentation for [[ https://secure.phabricator.com/conduit/method/differential.setdiffproperty/ | differential.setdiffproperty ]] is unclear. It should be updated by including the following information.

  1. What are the allowed values for name?
  2. For each allowed value for name:
    1. What is the intended meaning of the property with that name?
    2. What are the allowed values for data when this name is specified?
    3. Does the property impact the UI in any way, or is it only used internally?

In addition to this basic description, one or more examples showing a complete JSON message for this API call would be very helpful.

Event Timeline

sharwell created this task.Sep 3 2015, 3:24 PM
sharwell updated the task description. (Show Details)
sharwell added a project: Conduit.
sharwell added a subscriber: sharwell.

What are you trying to accomplish?

Short answer: We aren't sure yet.

Long answer:

We are considering using Conduit to enhance our overall experience. However, right now:

  1. We aren't sure what is possible with this API.
  2. We can't be sure that behavior we find experimentally will continue to behave this way in the future.

Providing the described documentation allows us to start forming our plan for using the API with confidence that it will work as a medium- to long-term solution and reduces the work required to reach these goals once we form them. I believe this task (discovery/documentation) makes sense here because all Phabricator users are able to benefit from the result.

📝 I filed this request for a single, smaller API intentionally. I'm hoping the chosen granularity will prove to be a very simple task for someone already familiar with the way the call works.

epriestley closed this task as Wontfix.Sep 3 2015, 4:51 PM
epriestley claimed this task.

It's very hard for us to prioritize work which doesn't actually solve real problems, or where we don't understand the greater need. This is why we ask users to describe problems and avoid hypotheticals in the guide describing how to file good feature requests.

Here are the answers to your questions, technically speaking:

  1. Any string value is accepted.
  2. Only lint-excuse and unit-excuse have any upstream meaning today.
    1. The excuses are shown next to failing lint and unit tests.
    2. Any value is permitted. The excuse keys take strings.
    3. Those keys are shown in the UI.

Knowing this doesn't help anyone do anything, and it is very hard for us to justify spending time on this instead of something else (which solves a real problem).

For more context, see:

This is a pretty old ticket, but if possible I'd like to add a use case:

I want to add a Project to a diff. Is this a property that can be set? If not, is therre an API that permits this?

avivey added a subscriber: avivey.Jan 12 2016, 3:38 AM

@joncalhoun: That's not related to this ticket, and also not supported. You can add projects to Revisions, but not Diffs. There will be an API for projects on Revisions eventually, see T5873.

benfiola added a subscriber: benfiola.EditedJan 29 2016, 6:22 PM

Sorry to necro an old thread, but I, too, am looking for information about this particular API method.

So, say I have a diff with an ID of 19. I want to add an arbitrary property to this diff, so I call differential.setdiffproperty with the following payload:

{
    "diff_id":19,
    "name":"test",
    "data":"test"
}

The Conduit response returns an error value of "null" and an errorMessage value of "null", so I assume it's been added.

When I call differential.querydiffs with the following payload :

{
   ids:[19]
}

I'm unable to find the property I've added anywhere within the diff data structure.

Am I fundamentally misunderstanding what a property means in this context, or am I querying for these diff properties incorrectly?

This is an arbitrary example. For a real-world example, I'm using a daemon to interface between Differential and another application and it'd be pretty convenient to store some metadata as properties of a diff review to prevent the daemon from performing the same work every time it's run. (Leaving comments, etc.) However, I felt that providing an arbitrary example would better illustrate the problem I'm having over an example that's a direct application of what I'm working on.