Re: [Starlingx-discuss] 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@windriver.com] *Sent:* Monday, December 3, 2018 11:14 PM *To:* Xie, Cindy <cindy.xie@intel.com>; Chen, Haochuan Z <haochuan.z.chen@intel.com>; Wold, Saul <saul.wold@intel.com>; Eslimi, Dariush <Dariush.Eslimi@windriver.com>; McKenna, Jason <Jason.McKenna@windriver.com> *Cc:* Liu, ZhipengS <zhipengs.liu@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Little, Scott <Scott.Little@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@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@windriver.com] *Sent:* Monday, December 3, 2018 10:45 PM *To:* Chen, Haochuan Z <haochuan.z.chen@intel.com <mailto:haochuan.z.chen@intel.com>>; Wold, Saul <saul.wold@intel.com <mailto:saul.wold@intel.com>>; Eslimi, Dariush <Dariush.Eslimi@windriver.com <mailto:Dariush.Eslimi@windriver.com>>; McKenna, Jason <Jason.McKenna@windriver.com <mailto:Jason.McKenna@windriver.com>> *Cc:* Liu, ZhipengS <zhipengs.liu@intel.com <mailto:zhipengs.liu@intel.com>>; Lin, Shuicheng <shuicheng.lin@intel.com <mailto:shuicheng.lin@intel.com>>; Xie, Cindy <cindy.xie@intel.com <mailto:cindy.xie@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@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
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@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@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@windriver.com] *Sent:* Monday, December 3, 2018 11:14 PM *To:* Xie, Cindy <cindy.xie@intel.com>; Chen, Haochuan Z <haochuan.z.chen@intel.com>; Wold, Saul <saul.wold@intel.com>; Eslimi, Dariush <Dariush.Eslimi@windriver.com>; McKenna, Jason <Jason.McKenna@windriver.com> *Cc:* Liu, ZhipengS <zhipengs.liu@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Little, Scott <Scott.Little@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@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@windriver.com] *Sent:* Monday, December 3, 2018 10:45 PM *To:* Chen, Haochuan Z <haochuan.z.chen@intel.com <mailto:haochuan.z.chen@intel.com>>; Wold, Saul <saul.wold@intel.com <mailto:saul.wold@intel.com>>; Eslimi, Dariush <Dariush.Eslimi@windriver.com <mailto:Dariush.Eslimi@windriver.com>>; McKenna, Jason <Jason.McKenna@windriver.com <mailto:Jason.McKenna@windriver.com>> *Cc:* Liu, ZhipengS <zhipengs.liu@intel.com <mailto:zhipengs.liu@intel.com>>; Lin, Shuicheng <shuicheng.lin@intel.com <mailto:shuicheng.lin@intel.com>>; Xie, Cindy <cindy.xie@intel.com <mailto:cindy.xie@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@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
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@windriver.com] Sent: Tuesday, December 4, 2018 1:21 AM To: Saul Wold <sgw@linux.intel.com>; Xie, Cindy <cindy.xie@intel.com>; Chen, Haochuan Z <haochuan.z.chen@intel.com>; Eslimi, Dariush <Dariush.Eslimi@windriver.com>; McKenna, Jason <Jason.McKenna@windriver.com>; starlingx-discuss@lists.starlingx.io Cc: Liu, ZhipengS <zhipengs.liu@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Little, Scott <Scott.Little@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@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@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@windriver.com] *Sent:* Monday, December 3, 2018 11:14 PM *To:* Xie, Cindy <cindy.xie@intel.com>; Chen, Haochuan Z <haochuan.z.chen@intel.com>; Wold, Saul <saul.wold@intel.com>; Eslimi, Dariush <Dariush.Eslimi@windriver.com>; McKenna, Jason <Jason.McKenna@windriver.com> *Cc:* Liu, ZhipengS <zhipengs.liu@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Little, Scott <Scott.Little@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@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@windriver.com] *Sent:* Monday, December 3, 2018 10:45 PM *To:* Chen, Haochuan Z <haochuan.z.chen@intel.com <mailto:haochuan.z.chen@intel.com>>; Wold, Saul <saul.wold@intel.com <mailto:saul.wold@intel.com>>; Eslimi, Dariush <Dariush.Eslimi@windriver.com <mailto:Dariush.Eslimi@windriver.com>>; McKenna, Jason <Jason.McKenna@windriver.com <mailto:Jason.McKenna@windriver.com>> *Cc:* Liu, ZhipengS <zhipengs.liu@intel.com <mailto:zhipengs.liu@intel.com>>; Lin, Shuicheng <shuicheng.lin@intel.com <mailto:shuicheng.lin@intel.com>>; Xie, Cindy <cindy.xie@intel.com <mailto:cindy.xie@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@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
participants (3)
-
Chen, Haochuan Z
-
Penney, Don
-
Saul Wold