GTG

Code review comment for lp:~nimit-svnit/gtg/dates

Revision history for this message
Nimit Shah (nimit-svnit) wrote :

   Yes that is correct. The problem is like this:
    trunk has revno = 1194 and my branch has revno=1190 and when I merge
and then commit the branch revno=1191 and not 1195. How to solve this
problem?

Nimit Shah,
B Tech 3rd year,
Computer Engineering Department,
SVNIT Surat
Secretary ACM-NIT Surat
www.dude-says.blogspot.com

On Fri, Jun 8, 2012 at 11:58 AM, Bertrand Rousseau <
<email address hidden>> wrote:

> On Fri, Jun 8, 2012 at 5:52 AM, Nimit Shah <email address hidden> wrote:
> > Hi!
> > I have been having this problem since long. Last time Bertrand did my
> > work for me. But this time, I wish to do it myself.
> > Whenever I do bzr merge ../trunk the revision number of my branch is
> > not updated. Can anyone tell me the probable cause?
>
> Yes, it's how bzr works: when you merge another branch in yours, it
> imports the change from this branch, but it let you commit them (which
> makes sens since sometimes you have to fix conflicts). Look at bzr
> documentation to know more about its workflow.
>
> > Another topic: Bertrand did you get a chance to read my last reply? Do
> > you agree with my points regarding the get_due_date method?
>
> I still have to some remarks, but I need more time.
>
> > Nimit Shah,
> > B Tech 3rd year,
> > Computer Engineering Department,
> > SVNIT Surat
> > Secretary ACM-NIT Surat
> > www.dude-says.blogspot.com
> >
> >
> >
> > On Thu, Jun 7, 2012 at 7:44 AM, Nimit Shah <email address hidden>
> wrote:
> >
> >> Hi Bertrand!
> >> Thank you for the review!
> >>
> >> On Thu, Jun 7, 2012 at 4:03 AM, Bertrand Rousseau <
> >> <email address hidden>> wrote:
> >>
> >>> Review: Needs Fixing code inspection
> >>>
> >>> Some remarks:
> >>>
> >>> (line numbers are references from your patch)
> >>>
> >>> - I suggest you move line 36 in top of the method code, and remove the
> >>> line 22. It seems to me that the current code will update the due_date
> two
> >>> times if the start date comes after the new due date
> >>>
> >>
> >> You are right. I had made a small modification earlier. I should have
> >> moved the line up after I removed the modification.
> >>
> >>> - in line 27, you seem to test if the parent has a due date by doing
> "if
> >>> par.due_date", shouldn't it be "if par.due_date != Date.no_date()"?
> >>> - same remark for the children start date in line 34 ("if
> >>> sub.start_date")
> >>>
> >>
> >> Regarding the par.due_date != Date.no_date(), I just tried "if
> >> par.due_date" and it was working for me, hence I thought of keeping it
> that
> >> way. Anyways, I will modify it as that will be better to understand.
> >>
> >>
> >>> - you should use the accessor functions when you need to get the
> >>> start/due date from other object than the current (parents and children
> >>> tasks)
> >>> - if you unset the due date (fulldate == Date.no_date()), shouldn't
> you
> >>> skip all parent/children updates? Right now, you skip only the parent
> update
> >>>
> >>
> >> fulldate == Date.no_date check was required because of the comparisons
> of
> >> the dates. If I simply kept the comparison "par.due_date <
> Date(fulldate)"
> >> then when I would clear the due_date of child, the due_date of parent
> would
> >> also be cleared. Hence I needed to keep this check.
> >>
> >>
> >> - you shouldn't change the due date accessor, it seems to me that the
> >>> previous behavior was correct.
> >>>
> >>
> >> I'll explain the reason I removed the due_date accessor and was not
> using
> >> get_due_date.
> >> Earlier I was using the due_date accessor and the old
> implementation.
> >> But the problem was, get_due_date returns the most urgent due_date
> amongst
> >> it's parents but doesnot set the child's due_date equal to it. So, after
> >> setting the parent's due_date, when I ran the loop for child. The loop
> >> would run uselessly because, the get_due_date would return a due_date
> same
> >> as that of parent (in my case). Hence, even though the actual due_date
> of
> >> child would be later, the get_due_date accessor's behaviour made it
> >> impossible to update it without directly accessing the date.
> >>
> >> If we use the my modified get_due_date then the real due_date would
> be
> >> returned which will make it possible to view actually modify the child.
> >>
> >>
> >>> --
> >>> https://code.launchpad.net/~nimit-svnit/gtg/dates/+merge/108785<
> https://code.launchpad.net/%7Enimit-svnit/gtg/dates/+merge/108785>
> >>> You are the owner of lp:~nimit-svnit/gtg/dates.
> >>>
> >>
> >>
> >> Nimit Shah,
> >> B Tech 3rd year,
> >> Computer Engineering Department,
> >> SVNIT Surat
> >> Secretary ACM-NIT Surat
> >> www.dude-says.blogspot.com
> >>
> >>
> >
> > --
> > https://code.launchpad.net/~nimit-svnit/gtg/dates/+merge/108785
> > You are reviewing the proposed merge of lp:~nimit-svnit/gtg/dates into
> lp:gtg.
>
>
>
> --
> Bertrand Rousseau
>
> https://code.launchpad.net/~nimit-svnit/gtg/dates/+merge/108785
> You are the owner of lp:~nimit-svnit/gtg/dates.
>

« Back to merge proposal