[Deepsea-users] DeepSea 0.7.6

[Joshua Schmid]

On 24/04/2017 10:27, Joao Eduardo Luis wrote:
> On 04/24/2017 03:20 AM, Joshua Schmid wrote:
>>
>>
>> On 23/04/2017 15:50, Joao Eduardo Luis wrote:
>>> On 04/22/2017 02:43 PM, Eric Jackson wrote:
>>>> Hello everyone,
>>>>   DeepSea 0.7.6 has been released.  The notable feature is the rolling
>>>> upgrade.  With a running Ceph cluster, an admin can gracefully upgrade
>>>> the OS,
>>>> Salt and Ceph.  See my previous email about specifics.  The CHANGELOG
>>>> is listed
>>>> below:
>>>>
>>>> - Rolling upgrade
>>>
>>> I must say I'm a bit surprised seeing this today, just a single day
>>> after you gave a "heads up about a large PR to implement the rolling
>>> upgrade".
>>>
>>> Has this behemoth of a PR, with "81 commits and 55 files [...] Several
>>> of these [... being ...] new" been properly peer-reviewed and discussed?
>>>
>>> I ask this because upon checking the PR on github, as well as the merge
>>> commit, I see no discussion or reviews/Reviewed-by.
>>>
>>> It feels strange seeing such a large PR, with roughly 1.4k added lines,
>>> to be announced one day and merged the very next; especially without
>>> seeing any sort of involvement from anyone else (beside the authors).
>>>
>>
>> The original commit #43 [https://github.com/SUSE/DeepSea/pull/43] has
>> quite some comments and was reviewed by Blain. It has been open for a
>> while now and plenty of discussions with various people have been held.
>> The fashion in which the Final PR #222 was merged doesn't look as well
>> reviewed, but it actually is only copy+addendum of #43/162.
> 
> I apologize for the noise then. It was not obvious such a discussion had
> been had.
> 
> In the past, I've found that in cases like these it's useful to have a
> pointer to the PR where the review process happened. If not for anything
> else, at least to allow the community around a project to have a sense
> of transparency. Not everyone follows development, but everyone likes to
> know where things are coming from when major milestones are reached.

the description of #222, points to #43 and #162 :)

https://github.com/SUSE/DeepSea/pull/222#issue-223528515

I think we had a discussion on using highlevel & non-interal labels for
milestones just like ceph upstream does. Couldn't find it in the
archives right now, but that would've helped to clarify the goals a bit
more. Reopening the discussion hereby.
> 
> 
>>> Additionally, I'm inclined to presume there were no other set of eyes on
>>> the PR due to commits such as
>>>
>>>   3dd3fe474df036ed4322b4d03da9d57934ac3baa
>>>
>>> which fixes a 'typo', and could have been squashed with the previous
>>> commit
>>>
>>>   7ca4dabbd2311a04eb39b03c2b29343970f7e476
>>>
>>> (which, in this case, would have reduced the number of commits in the
>>> patch set).
>>
>> +1. I should've squashed them before.
>> We haven't talked about a PR squashing habit in deepsea, yet. But I'm in
>> favor of enforcing such a rule.
> 
> Call me biased, but I believe this should not require a rule to be
> enforced - it should be common sense. Squashing makes reading the
> history easier, and allows you to review individual patches. If people
> don't squash, you're forced to review the whole diff.
> 
> Besides, not squashing patches like the one I mentioned leads to
> git-bisect being completely useless, because some of the patches won't
> even compile/run/wtv.
> 
>   -Joao
> _______________________________________________
> Deepsea-users mailing list
> Deepsea-users at lists.suse.com
> http://lists.suse.com/mailman/listinfo/deepsea-users

-- 
Freundliche Grüße - Kind regards,
Joshua Schmid
SUSE Enterprise Storage
SUSE Linux GmbH - Maxfeldstr. 5 - 90409 Nürnberg
--------------------------------------------------------------------------------------------------------------------
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Jennifer Guild, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nürnberg)
--------------------------------------------------------------------------------------------------------------------