From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.6.1597254010480520163 for ; Wed, 12 Aug 2020 10:40:10 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=A/xGY4XT; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1597254009; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UZPZIHkIwVJt5fV8CVHqJ4lTlp7eRE9vjHtzLxrdgRM=; b=A/xGY4XTpPD9jiRinFXgpPvLtpGCIX3pkGYsYNyL9TFJMIFhYHskOahrWXTlg+J/0oF9QY 649ckzdVYM3qCC+PeViFcgZpHG36R1Rv43gVFAZ3CpzWd+fmm6vTJO7UBAMq5v+qmvR+7N IUsPTAlGU9ob3lF+oZD/oiZv63agMYw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-248-Dt63Ihe4MsaqSWjCM1cEow-1; Wed, 12 Aug 2020 13:40:05 -0400 X-MC-Unique: Dt63Ihe4MsaqSWjCM1cEow-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 979F0106B252; Wed, 12 Aug 2020 17:40:03 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-34.ams2.redhat.com [10.36.114.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 301B319D71; Wed, 12 Aug 2020 17:40:00 +0000 (UTC) Subject: Re: [EXTERNAL] [edk2-devel] [PATCH v4] UefiCpuPkg/MtrrLib/UnitTest: Add host based unit test To: "Ni, Ray" , "devel@edk2.groups.io" , "Kinney, Michael D" , "tim.lewis@insyde.com" , "afish@apple.com" , "bret.barkelew@microsoft.com" Cc: "spbrogan@outlook.com" , "Shao, Ming" , "Dong, Eric" , 'Sean Brogan' , "Yao, Jiewen" References: <067501d6651a$eeda3040$cc8e90c0$@insyde.com> <1629DBB4A876976B.23512@groups.io> <734D49CCEBEEF84792F5B80ED585239D5C66C65F@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <4368c0ad-a781-825e-a99d-cffbf8609620@redhat.com> Date: Wed, 12 Aug 2020 19:40:00 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C66C65F@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 08/12/20 16:22, Ni, Ray wrote: > Sorry I somehow pushed the https://github.com/niruiyu/edk2/commit/a1ca847e0a4f7f679d8c28c31d55c84f304d7a87 together with this patch to upstream master. (That's probably commit 65904cdbb33c ("UefiCpuPkg/MtrrLibUnitTest: Change to use static array for CI test", 2020-08-12) now.) > The additional patch is to follow the suggestions in this thread, to make issues detected in CI reproduceable. > > I created a pull request to trigger personal build to verify whether the additional change is good to pass CI. But it seems the Github automatically merged this with the original pull request (https://github.com/tianocore/edk2/pull/827) and the two changes then were pushed together because the push label was set. If you pushed the new patch using the same branch that was the subject of the original pull request (labeled "push"), then github must have indeed taken the 2nd push as an "update" for the existent pull request. > The push label was set 16 days ago and the patch cannot be merged due to the CI failure at that moment. But this time CI is happy so the patches are merged. I don't like to leave PRs open for such a long time. > I plan: > > 1. send the second patches for review Sounds OK. > 2. revert the two patches Why is it necessary to revert the first commit too? Was it not reviewed either? > 3. add the Reviewed-by to the two patches and trigger the pull-request > Is that ok? Sure, in my opinion. I'd start with the revert though, as reviewing the additional patch might take extra time. Thanks Laszlo