patch for issue #952

Dirk Hohndel dirk at hohndel.org
Mon Jan 18 12:43:01 PST 2016


Thanks for your patch... a few comments...

First - I think patches and their discussion belong on the developer
mailing list, not on the user forum. So that's where I directed my
response.

Next, here's a bit of feedback on the commit:

a) issue #952 deals with an incorrect firmware issue on an Atomics Cobalt
dive computer. I'm guessing this is not what this patch is about.

b) please look into the formatting of your commit message.
https://subsurface-divelog.org/documentation/contributing/ has a nice
explanation how your commit message should be formatted. I would
especially like to have a full first and last name in the SOB line and a
clear, concise commit title and explanation of what your fix does in it's
body (with an empty line separating the title from the body).

c) as the last line of the commit message, before the SOB line (and with
empty lines before and after) you should have a reference to the bug that
you are fixing (which is actually 954).

So the commit message should look somehting like this:


==================
Fix UI issue with location management

The index in the fixpopup function was incorrect which caused the user to
need TWO cursor down presses to move to the second element.

Fixes #954

Signed-off-by: Firstname Lastname <email at address.tld>
==================


Woulf you please resubmit an updated commit? And thanks again for your
contribution! I'm trying to provide constructive feedback to make it
easier for you to contribute in the future.

Thanks

/D

On Tue, Jan 19, 2016 at 02:03:10AM +0530, Krishan Chopra wrote:
> This patch solves ticket 952. i sent an earlier patch for same issue.
> please do not consider it. my local branch was not updated so the patch had
> many extra things.
> consider this patch only.
> 
> -- 
> You received this message because you are subscribed to the Google Groups "Subsurface Divelog" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to subsurface-divelog+unsubscribe at googlegroups.com.
> To post to this group, send email to subsurface-divelog at googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/subsurface-divelog/CANCcaVk9FuANO1Q%3Dwk%3DafWw47TnXU2w1LGC_JKAiqW_ZiD7Tdw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.

> From 73fc143dd350a8a4d9193b98c35996753efb1998 Mon Sep 17 00:00:00 2001
> From: krisalpha <choprakrishan61 at gmail.com>
> Date: Tue, 19 Jan 2016 01:44:30 +0530
> Subject: [PATCH 2/2] This patch solves ticket #952 location Ui issue.
>  Earlier it took one extra keystroke to reach the second element. This problem
>  came for first time only. I changed index in fixpopup function in
>  locationinformation.cpp file.
> 
> Signed-off-by: krisalpha <choprakrishan61 at gmail.com>
> ---
>  desktop-widgets/locationinformation.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/desktop-widgets/locationinformation.cpp b/desktop-widgets/locationinformation.cpp
> index f23e5e8..2501ad7 100644
> --- a/desktop-widgets/locationinformation.cpp
> +++ b/desktop-widgets/locationinformation.cpp
> @@ -569,7 +569,7 @@ void DiveLocationLineEdit::fixPopupPosition()
>  
>  	view->setGeometry(pos.x(), pos.y(), w, h);
>  	if (!view->currentIndex().isValid() && view->model()->rowCount()) {
> -		view->setCurrentIndex(view->model()->index(0, 0));
> +		view->setCurrentIndex(view->model()->index(0, 1));
>  	}
>  }
>  
> -- 
> 2.1.4
> 



More information about the subsurface mailing list