User Details
- User Since
- Jun 29 2017, 2:56 PM (186 w, 4 d)
Today
Sat, Jan 23
Fri, Jan 22
Thu, Jan 21
I like the fact we now take a rev as argument. However two discussion remains:
Wed, Jan 20
Sad but useful.
Should this go on stable instead to make the test fixed on stable too?
Mon, Jan 18
@joerg.sonnenberger and I had a discussion on IRC about this API and the result is that Joerg prefers the higher level API for reason I now understand better without necessarly finding them decisive while I still prefer the lower level API for reason that (hopefully) Joerg understand better without finding them more decisive. So we are doing to need a bit more time to thing about that (and probably about the broader picture of the full API of the involved object) with probably more people.
So I am still not fan of this API:
I phabsent this in behalf of Victor to avoid him premature exposure to phabricator crapiness.
(this is otherwise great), and we can probably turn this in a property cache as a follow up.
This looks good, thanks.
Still not convinced with the API, so poking at what else would be possible.
I nothing obvious emerge we should move forward with that series before the freeze, the change final change is quite valuable.
This is clearer. For extra clarity I would probably use something like:
This looks good. The documentation could be a bit better. However this can be dealt with during the freeze. So lets focus on the implementation/behavior side for now.
The requested distinction happen as a follow up in D9824. It seems fine to me.
Am I fine with the required fix being a follow up and such follow up now exist. So I think we are good to go.
This looks "fine" as it fix the previous race window. However do not hesitate to follow up with another patch to reduce the code duplication.
Great, thanks.
Can you add a small in place comment to explain the requirements ?
Same as before, we will have two different option as one case (probably upgrade) migh be expected within an organization but not the other.
We need a distinct option for upgrading and downgrading. The "risk" associated with each operation is quite different and should be dealt with differently.
Sun, Jan 17
note: I am fine with D9783 being taken in its current forms and the locking being fixed as a follow up. Overall the series is still an improvement.