[Starlingx-discuss] review for story 2004211 patch

Tue Dec 4 00:57:39 UTC 2018

I could make clean build successfully after remove this patch for make check. We could double confirm with Jason, how the build error happens and the history.

After this patch removed, I could use sudo rpm, when I submit fix for this story, removing all patch for this package.
https://storyboard.openstack.org/#!/story/2004212

Martin, Chen
SSG OTC, Software Engineer
021-61164330

-----Original Message-----
From: Penney, Don [mailto:Don.Penney at windriver.com] 
Sent: Tuesday, December 4, 2018 1:21 AM
To: Saul Wold <sgw at linux.intel.com>; Xie, Cindy <cindy.xie at intel.com>; Chen, Haochuan Z <haochuan.z.chen at intel.com>; Eslimi, Dariush <Dariush.Eslimi at windriver.com>; McKenna, Jason <Jason.McKenna at windriver.com>; starlingx-discuss at lists.starlingx.io
Cc: Liu, ZhipengS <zhipengs.liu at intel.com>; Lin, Shuicheng <shuicheng.lin at intel.com>; Little, Scott <Scott.Little at windriver.com>
Subject: RE: review for story 2004211 patch

I think it's perfectly reasonable for me to ask these questions on reviews when I have them. And to repeat my questions when they're left unanswered on the review.

As for the storyboard, Dariush added the comment "Seems reasonable alternative to refactor mock. have you tried to see if it is still valid?", to which there was no answer. I don't know if he looked into the history or talked to Jason about it, and he's on vacation at the moment.

I still don't see a reason for removing just this patch. If there are no other modifications, and we're now able to move to the binary RPM, then great. Otherwise, I don't see why we're doing this. Hence the question.

I don't know what the issue was with the check, or whether we'd still be impacted. Which is why I added Jason McKenna to the review asking for comment.

Cheers,
Don.

-----Original Message-----
From: Saul Wold [mailto:sgw at linux.intel.com]
Sent: Monday, December 03, 2018 11:49 AM
To: Xie, Cindy; Penney, Don; Chen, Haochuan Z; Eslimi, Dariush; McKenna, Jason; starlingx-discuss at lists.starlingx.io
Cc: Liu, ZhipengS; Lin, Shuicheng; Little, Scott
Subject: Re: review for story 2004211 patch

+starlingx-discuss

Don,

Some additional background:

I created the initial storyboard and talked with Dariush, he is supposed to provide the background about a given patch and if it can be removed, I assume he would have checked on the history and talked with the authors. Once he gave the go ahead to try removal, Martin did the work of removing the patch and testing it in the CentOS build environment.

I will wait for more details from Martin as suggested by Cindy regarding testing.

Sau!

On 12/3/18 7:21 AM, Xie, Cindy wrote:
> Agree w/ you Don. Just read the patch and see like this is a 
> workaround to avoid “make check” fail in certain mock config… @ 
> Martin, can you please double check w/ the patch author on this? And 
> your test confirmed that without this patch, the build can be 
> successful in this mock config already?
> 
> Thx. - cindy
> 
> *From:* Penney, Don [mailto:Don.Penney at windriver.com]
> *Sent:* Monday, December 3, 2018 11:14 PM
> *To:* Xie, Cindy <cindy.xie at intel.com>; Chen, Haochuan Z 
> <haochuan.z.chen at intel.com>; Wold, Saul <saul.wold at intel.com>; Eslimi, 
> Dariush <Dariush.Eslimi at windriver.com>; McKenna, Jason 
> <Jason.McKenna at windriver.com>
> *Cc:* Liu, ZhipengS <zhipengs.liu at intel.com>; Lin, Shuicheng 
> <shuicheng.lin at intel.com>; Little, Scott <Scott.Little at windriver.com>
> *Subject:* RE: review for story 2004211 patch
> 
> I understand that goal, but this particular patch was added to deal 
> with a build issue at the time, presumably. Was this a consideration 
> when removal of this patch was decided upon? Was there any discussion 
> with Scott or Jason to see if the patch may still be required?
> 
> *From:*Xie, Cindy [mailto:cindy.xie at intel.com]
> *Sent:* Monday, December 03, 2018 10:08 AM
> *To:* Penney, Don; Chen, Haochuan Z; Wold, Saul; Eslimi, Dariush; 
> McKenna, Jason
> *Cc:* Liu, ZhipengS; Lin, Shuicheng
> *Subject:* RE: review for story 2004211 patch
> 
> Don,
> 
> The driving force is to reduce the number of patches that we have to 
> maintain. This is the goal for non-openstack distro sub-project team.
> 
> Understand that we are not able to reduce all the patches for this 
> particular package, thus we still need to use sRPM instead of binary 
> RPM. However, the goodness is that we do not need to re-base those 
> patches when we have to upgrade CentOS next time. The less patches we 
> carry, the less upgrade effort will be.
> 
> Thanks. - cindy
> 
> *From:* Penney, Don [mailto:Don.Penney at windriver.com]
> *Sent:* Monday, December 3, 2018 10:45 PM
> *To:* Chen, Haochuan Z <haochuan.z.chen at intel.com 
> <mailto:haochuan.z.chen at intel.com>>; Wold, Saul <saul.wold at intel.com 
> <mailto:saul.wold at intel.com>>; Eslimi, Dariush 
> <Dariush.Eslimi at windriver.com <mailto:Dariush.Eslimi at windriver.com>>;
> McKenna, Jason <Jason.McKenna at windriver.com 
> <mailto:Jason.McKenna at windriver.com>>
> *Cc:* Liu, ZhipengS <zhipengs.liu at intel.com 
> <mailto:zhipengs.liu at intel.com>>; Lin, Shuicheng 
> <shuicheng.lin at intel.com <mailto:shuicheng.lin at intel.com>>; Xie, Cindy 
> <cindy.xie at intel.com <mailto:cindy.xie at intel.com>>
> *Subject:* RE: review for story 2004211 patch
> 
> I’ll re-post my question here, since it wasn’t answered on the review
> itself:
> 
> What is the driver for removing this patch? Are you sure this is safe 
> to remove in all build environments? Presumably it was added because 
> of a build failure at the time. Adding Jason to comment on the history
> 
> You don’t seem to be removing all patches from the package, so that we 
> could switch to just using the binary, so why remove just this one?
> 
> *From:*Chen, Haochuan Z [mailto:haochuan.z.chen at intel.com]
> *Sent:* Monday, December 03, 2018 12:40 AM
> *To:* Wold, Saul; Eslimi, Dariush; Penney, Don; McKenna, Jason
> *Cc:* Liu, ZhipengS; Lin, Shuicheng; Xie, Cindy
> *Subject:* review for story 2004211 patch
> 
> Hi folks
> 
> I have submit a patch to eliminate a meta patch, which re-enable make 
> check in sudo package’s building.
> 
> https://review.openstack.org/#/c/621057/
> 
> Please help to review. And wait for your opinion.
> 
> Thanks!
> 
> Martin, Chen
> 
> SSG OTC, Software Engineer
> 
> 021-61164330
> 