[DIM DOCS PATCH 1/2] doc: update CI and pre-merge details in committer guidelines

[DIM DOCS PATCH 1/2] doc: update CI and pre-merge details in committer guidelines

[Jani Nikula]

Lots has happened in the CI front since the first version was added.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drm-intel.rst | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/drm-intel.rst b/drm-intel.rst
index c68949a41c95..baf48f459dd9 100644
--- a/drm-intel.rst
+++ b/drm-intel.rst
@@ -172,6 +172,8 @@ An inexhaustive list of details to check:
   `details on testing requirements
   <http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html>`_.
 
+* The patch series has passed CI pre-merge testing. See CI details below.
+
 * An open source userspace, reviewed and ready for merging by the upstream
   project, must be available for new kernel ABI. Please see `details on
   upstreaming requirements
@@ -186,11 +188,6 @@ An inexhaustive list of details to check:
   (or the author) stand a chance to fairly quickly understand what goes wrong if
   the commit is reported to cause a regression?
 
-* `checkpatch.pl` does not complain. (Some of the more subjective warnings may
-  be ignored at the committer's discretion.)
-
-* The patch does not introduce new `sparse` warnings.
-
 * When pushing someone else's patch you must add your own signed-off per
   http://developercertificate.org/. dim apply-branch should do this
   automatically for you.
@@ -244,8 +241,6 @@ On Confidence, Complexity, and Transparency
   you have involved enough people to feel comfortable if the justification for
   the commit is questioned afterwards.
 
-* Make sure pre-merge testing is completed successfully.
-
 On Rough Consensus
 ------------------
 
@@ -290,18 +285,34 @@ discussions happen in public forums, and make sure there's a searchable
 permanent record of any discussions for later reference. This means that for
 most things internal meetings are not the most suitable venue.
 
-Pre-Merge Testing
------------------
+Continuous Integration and Pre-Merge Testing
+--------------------------------------------
+
+The requirements for CI_ pre-merge testing are:
+
+* ``checkpatch.pl`` does not complain. (Some of the more subjective warnings may
+  be ignored at the committer's discretion.)
+
+* The patch does not introduce new ``sparse`` warnings.
+
+* Patch series must pass IGT Basic Acceptance Tests (BAT) on all the CI machines
+  without causing regressions.
+
+* Patch series must pass full IGT tests on CI shard machines without causing
+  regressions.
+
+The CI bots will send results to the patch author and intel-gfx for any patches
+tracked by patchwork. The results are also available on patchwork_ and the CI_
+site.
+
+Check CI failures and make sure any sporadic failures are a) pre-existing,
+and b) tracked in bugzilla. If there's anything dubious that you can't track
+down to pre-existing and tracked issues please don't push, but instead figure
+out what's going on.
 
-Our CI infrastructure is being built up and currently requirements for pre-merge
-testing are fairly simple:
+.. _CI: https://intel-gfx-ci.01.org/
 
-* All patches must past IGT Basic Acceptance Tests (BAT) on all the CI machines
-  without causing regressions.  The CI bots will send results to intel-gfx for
-  any patches tracked by patchwork. Check CI failures and make sure any sporadic
-  failures are a) pre-existing b) tracked in bugzilla. If there's anything
-  dubious that you can't track down to pre-existing&tracked issues please don't
-  push, but instead figure out what's going on.
+.. _patchwork: https://patchwork.freedesktop.org/project/intel-gfx/series/
 
 Tooling
 =======
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[DIM DOCS PATCH 2/2] doc: clarify what type of changes are acceptable at commit time

[Jani Nikula]

As a rule of thumb, don't change patches while committing.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drm-intel.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drm-intel.rst b/drm-intel.rst
index baf48f459dd9..ad8ff9739336 100644
--- a/drm-intel.rst
+++ b/drm-intel.rst
@@ -196,6 +196,13 @@ An inexhaustive list of details to check:
   coordinate with maintainers to avoid unnecessary pain with conflicts. Usually
   some explicit merges are needed to avoid git getting lost.
 
+* As a general rule, do not modify the patches while applying, apart from the
+  commit message. If the patch conflicts, or needs to be changed due to review,
+  have the author rebase, update and resend. Any change at this stage is a
+  potential issue bypassing CI.
+
+  At most, minor comment and whitespace tweaks are acceptable.
+
 On Confidence, Complexity, and Transparency
 -------------------------------------------
 
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 1/2] doc: update CI and pre-merge details in committer guidelines

[Daniel Vetter]

On Wed, Jun 27, 2018 at 5:13 PM, Jani Nikula <jani.nikula@intel.com> wrote:
> Lots has happened in the CI front since the first version was added.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drm-intel.rst | 45 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/drm-intel.rst b/drm-intel.rst
> index c68949a41c95..baf48f459dd9 100644
> --- a/drm-intel.rst
> +++ b/drm-intel.rst
> @@ -172,6 +172,8 @@ An inexhaustive list of details to check:
>    `details on testing requirements
>    <http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html>`_.
>
> +* The patch series has passed CI pre-merge testing. See CI details below.
> +
>  * An open source userspace, reviewed and ready for merging by the upstream
>    project, must be available for new kernel ABI. Please see `details on
>    upstreaming requirements
> @@ -186,11 +188,6 @@ An inexhaustive list of details to check:
>    (or the author) stand a chance to fairly quickly understand what goes wrong if
>    the commit is reported to cause a regression?
>
> -* `checkpatch.pl` does not complain. (Some of the more subjective warnings may
> -  be ignored at the committer's discretion.)
> -
> -* The patch does not introduce new `sparse` warnings.
> -
>  * When pushing someone else's patch you must add your own signed-off per
>    http://developercertificate.org/. dim apply-branch should do this
>    automatically for you.
> @@ -244,8 +241,6 @@ On Confidence, Complexity, and Transparency
>    you have involved enough people to feel comfortable if the justification for
>    the commit is questioned afterwards.
>
> -* Make sure pre-merge testing is completed successfully.
> -
>  On Rough Consensus
>  ------------------
>
> @@ -290,18 +285,34 @@ discussions happen in public forums, and make sure there's a searchable
>  permanent record of any discussions for later reference. This means that for
>  most things internal meetings are not the most suitable venue.
>
> -Pre-Merge Testing
> ------------------
> +Continuous Integration and Pre-Merge Testing
> +--------------------------------------------
> +
> +The requirements for CI_ pre-merge testing are:
> +
> +* ``checkpatch.pl`` does not complain. (Some of the more subjective warnings may
> +  be ignored at the committer's discretion.)
> +
> +* The patch does not introduce new ``sparse`` warnings.
> +
> +* Patch series must pass IGT Basic Acceptance Tests (BAT) on all the CI machines
> +  without causing regressions.
> +
> +* Patch series must pass full IGT tests on CI shard machines without causing
> +  regressions.

* Patch series must pass gpu piglit tests on all CI machines without
causing regressions.

Very recent addition, and thus far hasn't really resulted in
breakage/regression reports, but it's there&in production.

Otherwise lgtm, ack.
-Daniel

> +
> +The CI bots will send results to the patch author and intel-gfx for any patches
> +tracked by patchwork. The results are also available on patchwork_ and the CI_
> +site.
> +
> +Check CI failures and make sure any sporadic failures are a) pre-existing,
> +and b) tracked in bugzilla. If there's anything dubious that you can't track
> +down to pre-existing and tracked issues please don't push, but instead figure
> +out what's going on.
>
> -Our CI infrastructure is being built up and currently requirements for pre-merge
> -testing are fairly simple:
> +.. _CI: https://intel-gfx-ci.01.org/
>
> -* All patches must past IGT Basic Acceptance Tests (BAT) on all the CI machines
> -  without causing regressions.  The CI bots will send results to intel-gfx for
> -  any patches tracked by patchwork. Check CI failures and make sure any sporadic
> -  failures are a) pre-existing b) tracked in bugzilla. If there's anything
> -  dubious that you can't track down to pre-existing&tracked issues please don't
> -  push, but instead figure out what's going on.
> +.. _patchwork: https://patchwork.freedesktop.org/project/intel-gfx/series/
>
>  Tooling
>  =======
> --
> 2.11.0
>
> _______________________________________________
> dim-tools mailing list
> dim-tools@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 2/2] doc: clarify what type of changes are acceptable at commit time

[Daniel Vetter]

On Wed, Jun 27, 2018 at 5:13 PM, Jani Nikula <jani.nikula@intel.com> wrote:
> As a rule of thumb, don't change patches while committing.
>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drm-intel.rst | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drm-intel.rst b/drm-intel.rst
> index baf48f459dd9..ad8ff9739336 100644
> --- a/drm-intel.rst
> +++ b/drm-intel.rst
> @@ -196,6 +196,13 @@ An inexhaustive list of details to check:
>    coordinate with maintainers to avoid unnecessary pain with conflicts. Usually
>    some explicit merges are needed to avoid git getting lost.
>
> +* As a general rule, do not modify the patches while applying, apart from the
> +  commit message. If the patch conflicts, or needs to be changed due to review,
> +  have the author rebase, update and resend. Any change at this stage is a
> +  potential issue bypassing CI.
>
Should we also mention that merge conflicts need to be told to
maintainers, so that they can do a backmerge? Just because this blew
up recently for drm-misc ...
-Daniel

> +  At most, minor comment and whitespace tweaks are acceptable.
> +
>  On Confidence, Complexity, and Transparency
>  -------------------------------------------
>
> --
> 2.11.0
>
> _______________________________________________
> dim-tools mailing list
> dim-tools@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 1/2] doc: update CI and pre-merge details in committer guidelines

[Rodrigo Vivi]

On Wed, Jun 27, 2018 at 06:13:00PM +0300, Jani Nikula wrote:
> Lots has happened in the CI front since the first version was added.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drm-intel.rst | 45 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 28 insertions(+), 17 deletions(-)
> 
> diff --git a/drm-intel.rst b/drm-intel.rst
> index c68949a41c95..baf48f459dd9 100644
> --- a/drm-intel.rst
> +++ b/drm-intel.rst
> @@ -172,6 +172,8 @@ An inexhaustive list of details to check:
>    `details on testing requirements
>    <http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html>`_.
>  
> +* The patch series has passed CI pre-merge testing. See CI details below.
> +
>  * An open source userspace, reviewed and ready for merging by the upstream
>    project, must be available for new kernel ABI. Please see `details on
>    upstreaming requirements
> @@ -186,11 +188,6 @@ An inexhaustive list of details to check:
>    (or the author) stand a chance to fairly quickly understand what goes wrong if
>    the commit is reported to cause a regression?
>  
> -* `checkpatch.pl` does not complain. (Some of the more subjective warnings may
> -  be ignored at the committer's discretion.)
> -
> -* The patch does not introduce new `sparse` warnings.
> -
>  * When pushing someone else's patch you must add your own signed-off per
>    http://developercertificate.org/. dim apply-branch should do this
>    automatically for you.
> @@ -244,8 +241,6 @@ On Confidence, Complexity, and Transparency
>    you have involved enough people to feel comfortable if the justification for
>    the commit is questioned afterwards.
>  
> -* Make sure pre-merge testing is completed successfully.
> -
>  On Rough Consensus
>  ------------------
>  
> @@ -290,18 +285,34 @@ discussions happen in public forums, and make sure there's a searchable
>  permanent record of any discussions for later reference. This means that for
>  most things internal meetings are not the most suitable venue.
>  
> -Pre-Merge Testing
> ------------------
> +Continuous Integration and Pre-Merge Testing
> +--------------------------------------------
> +
> +The requirements for CI_ pre-merge testing are:
> +
> +* ``checkpatch.pl`` does not complain. (Some of the more subjective warnings may
> +  be ignored at the committer's discretion.)
> +
> +* The patch does not introduce new ``sparse`` warnings.
> +
> +* Patch series must pass IGT Basic Acceptance Tests (BAT) on all the CI machines
> +  without causing regressions.
> +
> +* Patch series must pass full IGT tests on CI shard machines without causing
> +  regressions.
> +
> +The CI bots will send results to the patch author and intel-gfx for any patches
> +tracked by patchwork. The results are also available on patchwork_ and the CI_
> +site.
> +
> +Check CI failures and make sure any sporadic failures are a) pre-existing,
> +and b) tracked in bugzilla. If there's anything dubious that you can't track
> +down to pre-existing and tracked issues please don't push, but instead figure
> +out what's going on.
>  
> -Our CI infrastructure is being built up and currently requirements for pre-merge
> -testing are fairly simple:
> +.. _CI: https://intel-gfx-ci.01.org/
>  
> -* All patches must past IGT Basic Acceptance Tests (BAT) on all the CI machines
> -  without causing regressions.  The CI bots will send results to intel-gfx for
> -  any patches tracked by patchwork. Check CI failures and make sure any sporadic
> -  failures are a) pre-existing b) tracked in bugzilla. If there's anything
> -  dubious that you can't track down to pre-existing&tracked issues please don't
> -  push, but instead figure out what's going on.
> +.. _patchwork: https://patchwork.freedesktop.org/project/intel-gfx/series/
>  
>  Tooling
>  =======
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 2/2] doc: clarify what type of changes are acceptable at commit time

[Rodrigo Vivi]

On Wed, Jun 27, 2018 at 06:13:01PM +0300, Jani Nikula wrote:
> As a rule of thumb, don't change patches while committing.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drm-intel.rst | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drm-intel.rst b/drm-intel.rst
> index baf48f459dd9..ad8ff9739336 100644
> --- a/drm-intel.rst
> +++ b/drm-intel.rst
> @@ -196,6 +196,13 @@ An inexhaustive list of details to check:
>    coordinate with maintainers to avoid unnecessary pain with conflicts. Usually
>    some explicit merges are needed to avoid git getting lost.
>  
> +* As a general rule, do not modify the patches while applying, apart from the
> +  commit message. If the patch conflicts, or needs to be changed due to review,
> +  have the author rebase, update and resend. Any change at this stage is a
> +  potential issue bypassing CI.
> +
> +  At most, minor comment and whitespace tweaks are acceptable.
> +
>  On Confidence, Complexity, and Transparency
>  -------------------------------------------
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 1/2] doc: update CI and pre-merge details in committer guidelines

[Jani Nikula]

On Wed, 27 Jun 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jun 27, 2018 at 5:13 PM, Jani Nikula <jani.nikula@intel.com> wrote:
>> Lots has happened in the CI front since the first version was added.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drm-intel.rst | 45 ++++++++++++++++++++++++++++-----------------
>>  1 file changed, 28 insertions(+), 17 deletions(-)
>>
>> diff --git a/drm-intel.rst b/drm-intel.rst
>> index c68949a41c95..baf48f459dd9 100644
>> --- a/drm-intel.rst
>> +++ b/drm-intel.rst
>> @@ -172,6 +172,8 @@ An inexhaustive list of details to check:
>>    `details on testing requirements
>>    <http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html>`_.
>>
>> +* The patch series has passed CI pre-merge testing. See CI details below.
>> +
>>  * An open source userspace, reviewed and ready for merging by the upstream
>>    project, must be available for new kernel ABI. Please see `details on
>>    upstreaming requirements
>> @@ -186,11 +188,6 @@ An inexhaustive list of details to check:
>>    (or the author) stand a chance to fairly quickly understand what goes wrong if
>>    the commit is reported to cause a regression?
>>
>> -* `checkpatch.pl` does not complain. (Some of the more subjective warnings may
>> -  be ignored at the committer's discretion.)
>> -
>> -* The patch does not introduce new `sparse` warnings.
>> -
>>  * When pushing someone else's patch you must add your own signed-off per
>>    http://developercertificate.org/. dim apply-branch should do this
>>    automatically for you.
>> @@ -244,8 +241,6 @@ On Confidence, Complexity, and Transparency
>>    you have involved enough people to feel comfortable if the justification for
>>    the commit is questioned afterwards.
>>
>> -* Make sure pre-merge testing is completed successfully.
>> -
>>  On Rough Consensus
>>  ------------------
>>
>> @@ -290,18 +285,34 @@ discussions happen in public forums, and make sure there's a searchable
>>  permanent record of any discussions for later reference. This means that for
>>  most things internal meetings are not the most suitable venue.
>>
>> -Pre-Merge Testing
>> ------------------
>> +Continuous Integration and Pre-Merge Testing
>> +--------------------------------------------
>> +
>> +The requirements for CI_ pre-merge testing are:
>> +
>> +* ``checkpatch.pl`` does not complain. (Some of the more subjective warnings may
>> +  be ignored at the committer's discretion.)
>> +
>> +* The patch does not introduce new ``sparse`` warnings.
>> +
>> +* Patch series must pass IGT Basic Acceptance Tests (BAT) on all the CI machines
>> +  without causing regressions.
>> +
>> +* Patch series must pass full IGT tests on CI shard machines without causing
>> +  regressions.
>
> * Patch series must pass gpu piglit tests on all CI machines without
> causing regressions.
>
> Very recent addition, and thus far hasn't really resulted in
> breakage/regression reports, but it's there&in production.

Copy-pasted that and pushed, thanks.

BR,
Jani.

>
> Otherwise lgtm, ack.
> -Daniel
>
>> +
>> +The CI bots will send results to the patch author and intel-gfx for any patches
>> +tracked by patchwork. The results are also available on patchwork_ and the CI_
>> +site.
>> +
>> +Check CI failures and make sure any sporadic failures are a) pre-existing,
>> +and b) tracked in bugzilla. If there's anything dubious that you can't track
>> +down to pre-existing and tracked issues please don't push, but instead figure
>> +out what's going on.
>>
>> -Our CI infrastructure is being built up and currently requirements for pre-merge
>> -testing are fairly simple:
>> +.. _CI: https://intel-gfx-ci.01.org/
>>
>> -* All patches must past IGT Basic Acceptance Tests (BAT) on all the CI machines
>> -  without causing regressions.  The CI bots will send results to intel-gfx for
>> -  any patches tracked by patchwork. Check CI failures and make sure any sporadic
>> -  failures are a) pre-existing b) tracked in bugzilla. If there's anything
>> -  dubious that you can't track down to pre-existing&tracked issues please don't
>> -  push, but instead figure out what's going on.
>> +.. _patchwork: https://patchwork.freedesktop.org/project/intel-gfx/series/
>>
>>  Tooling
>>  =======
>> --
>> 2.11.0
>>
>> _______________________________________________
>> dim-tools mailing list
>> dim-tools@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dim-tools

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 2/2] doc: clarify what type of changes are acceptable at commit time

[Jani Nikula]

On Wed, 27 Jun 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jun 27, 2018 at 5:13 PM, Jani Nikula <jani.nikula@intel.com> wrote:
>> As a rule of thumb, don't change patches while committing.
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drm-intel.rst | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drm-intel.rst b/drm-intel.rst
>> index baf48f459dd9..ad8ff9739336 100644
>> --- a/drm-intel.rst
>> +++ b/drm-intel.rst
>> @@ -196,6 +196,13 @@ An inexhaustive list of details to check:
>>    coordinate with maintainers to avoid unnecessary pain with conflicts. Usually
>>    some explicit merges are needed to avoid git getting lost.
>>
>> +* As a general rule, do not modify the patches while applying, apart from the
>> +  commit message. If the patch conflicts, or needs to be changed due to review,
>> +  have the author rebase, update and resend. Any change at this stage is a
>> +  potential issue bypassing CI.
>>
> Should we also mention that merge conflicts need to be told to
> maintainers, so that they can do a backmerge? Just because this blew
> up recently for drm-misc ...

Added bullet

* Please notify maintainers (IRC is fine) of new merge conflicts during drm-tip
  rebuild, so that they can do backmerges as needed.

and pushed to get this done. Please amend if that's not what you wanted.

Thanks,
Jani.


> -Daniel
>
>> +  At most, minor comment and whitespace tweaks are acceptable.
>> +
>>  On Confidence, Complexity, and Transparency
>>  -------------------------------------------
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> dim-tools mailing list
>> dim-tools@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dim-tools

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 2/2] doc: clarify what type of changes are acceptable at commit time

[Daniel Vetter]

On Thu, Jul 5, 2018 at 3:53 PM, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 27 Jun 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Jun 27, 2018 at 5:13 PM, Jani Nikula <jani.nikula@intel.com> wrote:
>>> As a rule of thumb, don't change patches while committing.
>>>
>>> Cc: Imre Deak <imre.deak@intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drm-intel.rst | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drm-intel.rst b/drm-intel.rst
>>> index baf48f459dd9..ad8ff9739336 100644
>>> --- a/drm-intel.rst
>>> +++ b/drm-intel.rst
>>> @@ -196,6 +196,13 @@ An inexhaustive list of details to check:
>>>    coordinate with maintainers to avoid unnecessary pain with conflicts. Usually
>>>    some explicit merges are needed to avoid git getting lost.
>>>
>>> +* As a general rule, do not modify the patches while applying, apart from the
>>> +  commit message. If the patch conflicts, or needs to be changed due to review,
>>> +  have the author rebase, update and resend. Any change at this stage is a
>>> +  potential issue bypassing CI.
>>>
>> Should we also mention that merge conflicts need to be told to
>> maintainers, so that they can do a backmerge? Just because this blew
>> up recently for drm-misc ...
>
> Added bullet
>
> * Please notify maintainers (IRC is fine) of new merge conflicts during drm-tip
>   rebuild, so that they can do backmerges as needed.
>
> and pushed to get this done. Please amend if that's not what you wanted.

Personally I preferred committers told me before they wrestled a patch
into submission because it missed a required patch, then wrestled
drm-tip into submission with a nasty conflict on top. Doing a
backmerge first, pushing 2nd is much simpler, and fits into the bullet
about not modifying patches.

I'd have added something like "Also don't modify patches due to
conflicts. Instead ask maintainers to backmerge your required baseline
patches first, since that avoids tons of headaches later on with
conflicts in integration trees and pull requests." Or something like
that.
-Daniel

>
> Thanks,
> Jani.
>
>
>> -Daniel
>>
>>> +  At most, minor comment and whitespace tweaks are acceptable.
>>> +
>>>  On Confidence, Complexity, and Transparency
>>>  -------------------------------------------
>>>
>>> --
>>> 2.11.0
>>>
>>> _______________________________________________
>>> dim-tools mailing list
>>> dim-tools@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dim-tools
>
> --
> Jani Nikula, Intel Open Source Graphics Center



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [DIM DOCS PATCH 2/2] doc: clarify what type of changes are acceptable at commit time

[Jani Nikula]

On Thu, 05 Jul 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Jul 5, 2018 at 3:53 PM, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Wed, 27 Jun 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> On Wed, Jun 27, 2018 at 5:13 PM, Jani Nikula <jani.nikula@intel.com> wrote:
>>>> As a rule of thumb, don't change patches while committing.
>>>>
>>>> Cc: Imre Deak <imre.deak@intel.com>
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>> ---
>>>>  drm-intel.rst | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drm-intel.rst b/drm-intel.rst
>>>> index baf48f459dd9..ad8ff9739336 100644
>>>> --- a/drm-intel.rst
>>>> +++ b/drm-intel.rst
>>>> @@ -196,6 +196,13 @@ An inexhaustive list of details to check:
>>>>    coordinate with maintainers to avoid unnecessary pain with conflicts. Usually
>>>>    some explicit merges are needed to avoid git getting lost.
>>>>
>>>> +* As a general rule, do not modify the patches while applying, apart from the
>>>> +  commit message. If the patch conflicts, or needs to be changed due to review,
>>>> +  have the author rebase, update and resend. Any change at this stage is a
>>>> +  potential issue bypassing CI.
>>>>
>>> Should we also mention that merge conflicts need to be told to
>>> maintainers, so that they can do a backmerge? Just because this blew
>>> up recently for drm-misc ...
>>
>> Added bullet
>>
>> * Please notify maintainers (IRC is fine) of new merge conflicts during drm-tip
>>   rebuild, so that they can do backmerges as needed.
>>
>> and pushed to get this done. Please amend if that's not what you wanted.
>
> Personally I preferred committers told me before they wrestled a patch
> into submission because it missed a required patch, then wrestled
> drm-tip into submission with a nasty conflict on top. Doing a
> backmerge first, pushing 2nd is much simpler, and fits into the bullet
> about not modifying patches.
>
> I'd have added something like "Also don't modify patches due to
> conflicts. Instead ask maintainers to backmerge your required baseline
> patches first, since that avoids tons of headaches later on with
> conflicts in integration trees and pull requests." Or something like
> that.

Oh, I misunderstood you, in part because one of my original bullets
already says don't modify patches due to conflicts. I thought with
"merge conflicts" you specifically meant drm-tip conflicts.

BR,
Jani.



> -Daniel
>
>>
>> Thanks,
>> Jani.
>>
>>
>>> -Daniel
>>>
>>>> +  At most, minor comment and whitespace tweaks are acceptable.
>>>> +
>>>>  On Confidence, Complexity, and Transparency
>>>>  -------------------------------------------
>>>>
>>>> --
>>>> 2.11.0
>>>>
>>>> _______________________________________________
>>>> dim-tools mailing list
>>>> dim-tools@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dim-tools
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx