| Message ID | 20180627151301.9674-2-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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
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 -------------------------------------------
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(+)