On Sat, Mar 14, 2015 at 09:12:40PM +0000, David Gee wrote:
> Hi,
>
> The patch attached to my previous email contained an error. Somehow I'd
> inadvertently removed the 'x' from an xwimp call which I didn't intend
> to change.
The general (hand wave) way to mark such "please give feedback" patches
is to have [RFC] at the beginning of the subject, that way they are
easy to identify and you attract review comments without any danger of
the patch actually being applied.
It is also generally good etiquette to add [PATCH] if it is a patch you
actually want committed and [PULL] for git pull requests.
For some good hints on patch submission the Linux kernel
SubmittingPatches [1] document is a reasonable start. Though you can
skip all the sign off stuff, I mention it purely as a reasonable guide
(and in fact now I think about it I shall write one for NetSurf
proper)
Until the developers get used to you submitting patches (and
eventually grant you commit access) we are going to be a bit wary of
applying these changes without at least a couple of other people
chiming in with "yeah that looks OK" and or "that works for me" given
this is for RISC OS if Steve F says yes I will immediately apply it,
otherwise I am looking for confidence it fixes an issue without
introducing more.
Do you have a bug tracker login? [2] if so please let me know what it
is so i can elevate it to developer level. You will then be able to
get bugs assigned to you which should stop conflicts and show other
developers you are working on a bug.
>
> I've used git to revert the changes and then (once again) removed the
> xwimp_create_menu(WIMP_CLOSE_MENU,0,0) call from
> ro_gui_send_datasave--the change I originally intended to make. So the
> patch attached here should be sufficient on its own.
>
> Thanks to Jeremy Nicoll for pointing this out.
>
>
> --
> David Gee
> Gateshead
>
> >From 43889fdbc04b88b208ac676186d0773d3ffa4715 Mon Sep 17 00:00:00 2001
> From: David Gee <dr_d_...@blueyonder.co.uk>
> Date: Sat, 14 Mar 2015 20:28:49 +0000
> Subject: [PATCH] Change to save.c so menu tree is not needlessly collapsed on
> send_datasave.
>
your subject could do with a bit of work. I hope you do not mind the
feedback it is supposed to be positive and make applying your changes
easier for us.
You do not need to mention the source files changed, that comes from
the patch itself and we know it changes something ;-) This can take a
while to get used to but perhaps something like:
Subject: [PATCH] Stop RISC OS menu tree being closed on a datasave message.
and then a newline and then a brief paragraph explaining the logic
behind the change. See some of my commits like [3]
Sometimes the short summary is enough for simple changes but it is
good practice to get into the habit of doing it right when you are
starting.
> ---
> riscos/save.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/riscos/save.c b/riscos/save.c
> index b5d96c4..c4fd839 100644
> --- a/riscos/save.c
> +++ b/riscos/save.c
> @@ -723,7 +723,7 @@ void ro_gui_send_datasave(gui_save_type save_type,
> {
> /* Close the save window because otherwise we need two contexts
> */
> - xwimp_create_menu(wimp_CLOSE_MENU, 0, 0);
> +
> ro_gui_dialog_close(dialog_saveas);
>
> if (ro_message_send_message(wimp_USER_MESSAGE_RECORDED,
> (wimp_message*)message,
> --
> 1.9.1
>
[1] https://www.kernel.org/doc/Documentation/SubmittingPatches
[2] http://bugs.netsurf-browser.org/mantis/
[2]
http://git.netsurf-browser.org/netsurf.git/commit/?id=c4e551cd0cecf4ec9aba2d033cba3ca97e669463
--
Regards Vincent
http://www.kyllikki.org/
