Re: Kopete & KF5 switch

[vijay krishnavanshi]

Hi,


> Montel, Vijay, do you think also that KF5 version is really ready for
> release
> into KDE Applications 17.12?


Not sure. Command-line argument function and history plugin part needs to
be fixed (but they are not the primary part of the application). I am
working on it but because of university curriculum I get very little time :(
I will try to submit the patch by the coming week.

Regards,
Vijay Krishnavanshi

Re: Kopete & KF5 switch

[Pali Rohár]

On Thursday 09 November 2017 11:38:19 Luigi Toscano wrote:
> Pali Rohár ha scritto:
> > On Monday 07 August 2017 10:43:17 Pali Rohár wrote:
> >> On Monday 07 August 2017 10:26:46 Luigi Toscano wrote:
> >>> Il 07 agosto 2017 09:34:37 CEST, "Pali Rohár"  ha 
> >>> scritto:
> >>>
> 
>  Anyway, I rebased kf5 branch on top of master with (semi-manual)
>  conflicts resolution and found out that changes between kf5 branch and
>  "kf5 rebased on top of master" differs. And I suspect that some merge
>  commits which was done from master to kf5 were incorrectly.
> >>>
> >>> Do you mean merged, not rebased, right?
> >>
> >> I mean real git rebase but result in new temporary branch.
> > 
> > Hi! Now I rebased current kf5 branch on top of current master and pushed
> > it into my cloned git repository. Available at:
> > 
> > https://cgit.kde.org/clones/kopete/pali/kopete.git/?h=master-kf5
> > 
> > Reason for it: in kf5 branch are some older version of patches (with
> > errors) which were properly fixed in version of patches, which then were
> > integrated into master branch. Having periodical merge from master into
> > kf5 made 1) git history hard to interpret and 2) inability to use either
> > git bisect or git blame. It is really a bad idea to have two different
> > versions of one patch in two different branches in git which are then
> > merged...
> > 
> > Now diff between above my branch (master-kf5) and kf5 seems to be clean
> > and I would propose to use this branch which would be merged into master
> > (instead of original kf5).
> 
> Not really a fan of that, as it's really faking the history. Yes, wrong
> history on that way, but it's what it is.
> On the other side, there is no more time to discuss, as the dependency freeze
> (minus exceptions) for Applications 17.12 is today. So if you think that this
> Frameworks-based branch is working at least not worse than the kdelibs 4.x
> master, please go with the merge, or ask for an exception on 
> release-t...@kde.org
> 

Montel, Vijay, do you think also that KF5 version is really ready for release
into KDE Applications 17.12?

-- 
Pali Rohár
pali.ro...@gmail.com

Re: Kopete & KF5 switch

[Luigi Toscano]

Pali Rohár ha scritto:
> On Monday 07 August 2017 10:43:17 Pali Rohár wrote:
>> On Monday 07 August 2017 10:26:46 Luigi Toscano wrote:
>>> Il 07 agosto 2017 09:34:37 CEST, "Pali Rohár"  ha 
>>> scritto:
>>>

 Anyway, I rebased kf5 branch on top of master with (semi-manual)
 conflicts resolution and found out that changes between kf5 branch and
 "kf5 rebased on top of master" differs. And I suspect that some merge
 commits which was done from master to kf5 were incorrectly.
>>>
>>> Do you mean merged, not rebased, right?
>>
>> I mean real git rebase but result in new temporary branch.
> 
> Hi! Now I rebased current kf5 branch on top of current master and pushed
> it into my cloned git repository. Available at:
> 
> https://cgit.kde.org/clones/kopete/pali/kopete.git/?h=master-kf5
> 
> Reason for it: in kf5 branch are some older version of patches (with
> errors) which were properly fixed in version of patches, which then were
> integrated into master branch. Having periodical merge from master into
> kf5 made 1) git history hard to interpret and 2) inability to use either
> git bisect or git blame. It is really a bad idea to have two different
> versions of one patch in two different branches in git which are then
> merged...
> 
> Now diff between above my branch (master-kf5) and kf5 seems to be clean
> and I would propose to use this branch which would be merged into master
> (instead of original kf5).

Not really a fan of that, as it's really faking the history. Yes, wrong
history on that way, but it's what it is.
On the other side, there is no more time to discuss, as the dependency freeze
(minus exceptions) for Applications 17.12 is today. So if you think that this
Frameworks-based branch is working at least not worse than the kdelibs 4.x
master, please go with the merge, or ask for an exception on 
release-t...@kde.org

-- 
Luigi

Re: Kopete & KF5 switch

[Pali Rohár]

On Monday 07 August 2017 10:43:17 Pali Rohár wrote:
> On Monday 07 August 2017 10:26:46 Luigi Toscano wrote:
> > Il 07 agosto 2017 09:34:37 CEST, "Pali Rohár"  ha 
> > scritto:
> > 
> > >
> > >Anyway, I rebased kf5 branch on top of master with (semi-manual)
> > >conflicts resolution and found out that changes between kf5 branch and
> > >"kf5 rebased on top of master" differs. And I suspect that some merge
> > >commits which was done from master to kf5 were incorrectly.
> > 
> > Do you mean merged, not rebased, right?
> 
> I mean real git rebase but result in new temporary branch.

Hi! Now I rebased current kf5 branch on top of current master and pushed
it into my cloned git repository. Available at:

https://cgit.kde.org/clones/kopete/pali/kopete.git/?h=master-kf5

Reason for it: in kf5 branch are some older version of patches (with
errors) which were properly fixed in version of patches, which then were
integrated into master branch. Having periodical merge from master into
kf5 made 1) git history hard to interpret and 2) inability to use either
git bisect or git blame. It is really a bad idea to have two different
versions of one patch in two different branches in git which are then
merged...

Now diff between above my branch (master-kf5) and kf5 seems to be clean
and I would propose to use this branch which would be merged into master
(instead of original kf5).

-- 
Pali Rohár
pali.ro...@gmail.com

Re: Kopete & KF5 switch

[vijay krishnavanshi]

Hi,

I looked into the diff and I see following things like -

- Issue with the history plugin - I have addressed it with this review
request
Link - https://phabricator.kde.org/D7635
History plugin is revived as well and is working correctly after this RR.
Though some issues need to be fixed on Import History Part. I am working on
it.

- Issue with the external libiris bundle - changes in this are all of
dropping off the not required files and unused headers so we can drop off
all the changed in this and it will still work fine. I don't know yet how
to drop specific changes of the file or a directory. I will look it up and
will submit a patch.

I have tested it by building it on my system and checking it out on my
account of Gmail. It works correctly.

Regards,
Vijay Krishnavanshi


On 19 August 2017 at 21:33, vijay krishnavanshi <
vijaykrishnavan...@gmail.com> wrote:

> Hi,
>
> I was asking for force pushing the new branch in your clone repo. I am
> thinking how can the issues in the rebase branch be addressed.
>
> Another thing I want to ask is that why do we need this rebased branch,
> all the code is already there in the master branch and gives a working
> executable. Why address the merge conflicts? Is it because we want linear
> history or something else entirely.
>
> Regards,
> Vijay Krishnavanshi
>
> On 19 August 2017 at 21:24, Pali Rohár  wrote:
>
>> There is one problem with kf5 branch and merges... kf5 branch contains
>> more commits which were later applied into master branch, but slightly
>> different (e.g. in master branch were fixed problems). This results
>> probably in those merge conflicts which were somehow fixed.
>>
>> The whole rebase which I did into new branch is without preserving merge
>> commits and taking commits from master branch if one with "same commit
>> message" was also in kf5 branch (as that one in master branch is
>> correct).
>>
>> When you work, you should never do force pushes into main kopete
>> repository. I think that force pushes are also disabled on kde servers
>> for main kopete repository.
>>
>> In own cloned repositories, owner of clone can do what he wants, it is
>> his own copy.
>>
>> On Saturday 19 August 2017 21:03:28 vijay krishnavanshi wrote:
>> > Should I force push my branch also? Sorry for not tagging the
>> kopete-devel
>> > in above email.
>> >
>> > Regards,
>> > Vijay Krishnavanshi
>> >
>> > On 19 August 2017 at 21:01, vijay krishnavanshi <
>> > vijaykrishnavan...@gmail.com> wrote:
>> >
>> > > Okay,
>> > > I am working on it and trying to fix the issues in it. Diff is
>> because of
>> > > the merges I tried doing rebase of kf5 on master by using the command
>> "git
>> > > rebase master --preserve-merges". After fixing the issues in this it
>> leaves
>> > > me with the branch and non significant changes between the rebased
>> branch
>> > > and kf5 one.
>> > > --preserve-merges saves the merge commits also leaving us with less
>> issues
>> > > to be resolved. I might be wrong also but as of now I don't see any
>> build
>> > > issues.
>> > >
>> > > Regards,
>> > > Vijay Krishnavanshi
>> > >
>> > > On 19-Aug-2017 11:47, "Pali Rohár"  wrote:
>> > >
>> > >> There was some problem with this clone repository, kde sysadmin
>> fixed it
>> > >>
>> > >> https://cgit.kde.org/clones/kopete/pali/kopete.git/log/?h=master-kf5
>> > >>
>> > >>
>> > >>
>>
>> --
>> Pali Rohár
>> pali.ro...@gmail.com
>>
>
>

Re: Kopete & KF5 switch

[Pali Rohár]

There was some problem with this clone repository, kde sysadmin fixed it

https://cgit.kde.org/clones/kopete/pali/kopete.git/log/?h=master-kf5

On Wednesday 09 August 2017 22:57:04 Pali Rohár wrote:
> Hi! Try direct clone:
> 
> git clone g...@git.kde.org:clones/kopete/pali/kopete.git
> 
> and then checkout branch master-kf5
> 
> Looks like some problem with git kde infra...
> 
> On Thursday 10 August 2017 01:37:32 vijay krishnavanshi wrote:
> > Hi Pali,
> > 
> > I am still unable to access the repo. Can you please see if the link that
> > you gave is correct?
> > I don't think syncing should take this long, I may be wrong also.
> > 
> > Regards,
> > Vijay Krishnavanshi
> > 
> > 
> > New branch master-kf5 is in my cloned repository, available at:
> > > https://cgit.kde.org/clones/kopete/pali/kopete.git/log/?h=master-kf5
> > >
> > > diff between kf5 and my master-kf5 show some changes, so these needs to
> > > be investigated. Specially history plugin and jabber protocol.
> > >
> > > PS: If you do not see that cloned repository yet, just wait as at time
> > > of writing this email it was not synced to all kde git mirrors...
> > >
> > > --
> > > Pali Rohár
> > > pali.ro...@gmail.com
> > >
> 

-- 
Pali Rohár
pali.ro...@gmail.com

Re: Kopete & KF5 switch

[Pali Rohár]

Hi! Try direct clone:

git clone g...@git.kde.org:clones/kopete/pali/kopete.git

and then checkout branch master-kf5

Looks like some problem with git kde infra...

On Thursday 10 August 2017 01:37:32 vijay krishnavanshi wrote:
> Hi Pali,
> 
> I am still unable to access the repo. Can you please see if the link that
> you gave is correct?
> I don't think syncing should take this long, I may be wrong also.
> 
> Regards,
> Vijay Krishnavanshi
> 
> 
> New branch master-kf5 is in my cloned repository, available at:
> > https://cgit.kde.org/clones/kopete/pali/kopete.git/log/?h=master-kf5
> >
> > diff between kf5 and my master-kf5 show some changes, so these needs to
> > be investigated. Specially history plugin and jabber protocol.
> >
> > PS: If you do not see that cloned repository yet, just wait as at time
> > of writing this email it was not synced to all kde git mirrors...
> >
> > --
> > Pali Rohár
> > pali.ro...@gmail.com
> >

-- 
Pali Rohár
pali.ro...@gmail.com

Re: Kopete & KF5 switch

[vijay krishnavanshi]

Hi Pali,

I am still unable to access the repo. Can you please see if the link that
you gave is correct?
I don't think syncing should take this long, I may be wrong also.

Regards,
Vijay Krishnavanshi


New branch master-kf5 is in my cloned repository, available at:
> https://cgit.kde.org/clones/kopete/pali/kopete.git/log/?h=master-kf5
>
> diff between kf5 and my master-kf5 show some changes, so these needs to
> be investigated. Specially history plugin and jabber protocol.
>
> PS: If you do not see that cloned repository yet, just wait as at time
> of writing this email it was not synced to all kde git mirrors...
>
> --
> Pali Rohár
> pali.ro...@gmail.com
>

Re: Kopete & KF5 switch

[Pali Rohár]

On Monday 07 August 2017 09:34:37 Pali Rohár wrote:
> On Monday 07 August 2017 07:52:39 vijay krishnavanshi wrote:
> > Hi,
> > 
> > I am fixing the raised issues in old review requests so how do I submit the
> > changes for review. Should I create a new review request addressing all the
> > issues or create a separate review request for each old review request? or
> > comment on the old review request for the changes?
> > 
> > Regards,
> > Vijay Krishnavanshi
> 
> Hi! Choose solution which you like most. Ideally write comments to open
> issues either with proposed fix or with link to patch or a new request.
> 
> I would then either apply fix for current review request or take a new
> patch from a new review request.
> 
> Anyway, I rebased kf5 branch on top of master with (semi-manual)
> conflicts resolution and found out that changes between kf5 branch and
> "kf5 rebased on top of master" differs. And I suspect that some merge
> commits which was done from master to kf5 were incorrectly.
> 
> I will push that new branch (kf5 rebased on top of master) to kopete git
> repository under name master-kf5, so we would be able to compare and fix
> possible problems...

New branch master-kf5 is in my cloned repository, available at:
https://cgit.kde.org/clones/kopete/pali/kopete.git/log/?h=master-kf5

diff between kf5 and my master-kf5 show some changes, so these needs to
be investigated. Specially history plugin and jabber protocol.

PS: If you do not see that cloned repository yet, just wait as at time
of writing this email it was not synced to all kde git mirrors...

-- 
Pali Rohár
pali.ro...@gmail.com

Re: Kopete & KF5 switch

[Pali Rohár]

On Monday 07 August 2017 10:26:46 Luigi Toscano wrote:
> Il 07 agosto 2017 09:34:37 CEST, "Pali Rohár"  ha 
> scritto:
> 
> >
> >Anyway, I rebased kf5 branch on top of master with (semi-manual)
> >conflicts resolution and found out that changes between kf5 branch and
> >"kf5 rebased on top of master" differs. And I suspect that some merge
> >commits which was done from master to kf5 were incorrectly.
> 
> Do you mean merged, not rebased, right?

I mean real git rebase but result in new temporary branch.

> We should never force-rebase, and the pushing a new rebased branch would 
> remove the proper temporal reference.
> Please fix the merged branch instead.

But first we need to know what needs to be fixed. And for this purpose I
would push this new (temporary) branch so other people would be able to
see git diff between them.

-- 
Pali Rohár
pali.ro...@gmail.com

Re: Kopete & KF5 switch

[Luigi Toscano]

Il 07 agosto 2017 09:34:37 CEST, "Pali Rohár"  ha scritto:

>
>Anyway, I rebased kf5 branch on top of master with (semi-manual)
>conflicts resolution and found out that changes between kf5 branch and
>"kf5 rebased on top of master" differs. And I suspect that some merge
>commits which was done from master to kf5 were incorrectly.

Do you mean merged, not rebased, right? We should never force-rebase, and the 
pushing a new rebased branch would remove the proper temporal reference.
Please fix the merged branch instead.


-- 
Luigi

Re: Kopete & KF5 switch

[Pali Rohár]

On Monday 07 August 2017 07:52:39 vijay krishnavanshi wrote:
> Hi,
> 
> I am fixing the raised issues in old review requests so how do I submit the
> changes for review. Should I create a new review request addressing all the
> issues or create a separate review request for each old review request? or
> comment on the old review request for the changes?
> 
> Regards,
> Vijay Krishnavanshi

Hi! Choose solution which you like most. Ideally write comments to open
issues either with proposed fix or with link to patch or a new request.

I would then either apply fix for current review request or take a new
patch from a new review request.

Anyway, I rebased kf5 branch on top of master with (semi-manual)
conflicts resolution and found out that changes between kf5 branch and
"kf5 rebased on top of master" differs. And I suspect that some merge
commits which was done from master to kf5 were incorrectly.

I will push that new branch (kf5 rebased on top of master) to kopete git
repository under name master-kf5, so we would be able to compare and fix
possible problems...

-- 
Pali Rohár
pali.ro...@gmail.com

Re: Kopete & KF5 switch

[vijay krishnavanshi]

Hi,

I am fixing the raised issues in old review requests so how do I submit the
changes for review. Should I create a new review request addressing all the
issues or create a separate review request for each old review request? or
comment on the old review request for the changes?

Regards,
Vijay Krishnavanshi

Re: Kopete & KF5 switch

[vijay krishnavanshi]

Hi,


> > This commit looks okay to me as nothing much is changed in terms of
> > behavior. In this commit an attempt to close all the windows is made
> > when we are able to close all window other wise return from the
> > function. In normal conditions returning in the middle of for loop
> > does not cause any problem but I will try to verify the same for the
> > application. I have linked it below to make sure if this is the same
> > commit we are talking about.
> > Link :
> > https://cgit.kde.org/kopete.git/commit/?h=kf5=cb97ce4e61baa44d9f40
> > 1b8853982355ef6e684f
>
> Previous behaviour is to stay kopete running in systray even last
> windows is closed. And above commit change behavior from
> setQuitOnLastWindowClosed(false) to setQuitOnLastWindowClosed(true)
> which means that Kopete is no longer running in systray after closing
> last window.
>

Yes, now I see, I will make a patch undoing that particular line.

Regards,
Vijay Krishnavanshi

Re: Kopete & KF5 switch

[Pali Rohár]

On Sunday 06 August 2017 03:30:35 vijay krishnavanshi wrote:
> Hi,
> 
> > > 1) kf5 branch contains some commits which have open issues on
> > > reviewboard, namely:
> > > 
> > > https://git.reviewboard.kde.org/r/124305/
> > > https://git.reviewboard.kde.org/r/124181/
> > > https://git.reviewboard.kde.org/r/124349/
> 
> I will look into these changes and fix whatever necessary
> 
> > > 2) kf5 branch contains modification of source code of external
> > > bundled libiris library in kopete/protocols/jabber/libiris/. See
> > > also file: protocols/jabber/libiris/README_BEFORE_COMMITTING
> > > 
> > > $ git diff origin/master..origin/kf5 -- protocols/jabber/libiris/
> > > 
> > > Due to synchronization with upstream, we either need to drop
> > > these changes or send them to upstream.
> 
> These changes can be dropped and most of the changes are of using
> nullptr instead of 0. I will look into this more and give an update
> to what will be affected if it is dropped.
> 
> > And 3)
> > 
> > I'm not sure if commit cb97ce4e61baa44d9f401b8853982355ef6e684f
> > (Gentle quitting Kopete application) is correct.
> > 
> > It just looks like a workaround some bug in system try area.
> 
> This commit looks okay to me as nothing much is changed in terms of
> behavior. In this commit an attempt to close all the windows is made
> when we are able to close all window other wise return from the
> function. In normal conditions returning in the middle of for loop
> does not cause any problem but I will try to verify the same for the
> application. I have linked it below to make sure if this is the same
> commit we are talking about.
> Link :
> https://cgit.kde.org/kopete.git/commit/?h=kf5=cb97ce4e61baa44d9f40
> 1b8853982355ef6e684f

Previous behaviour is to stay kopete running in systray even last 
windows is closed. And above commit change behavior from 
setQuitOnLastWindowClosed(false) to setQuitOnLastWindowClosed(true) 
which means that Kopete is no longer running in systray after closing 
last window.

That second part of changing break to return is probably OK.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: Kopete & KF5 switch

[vijay krishnavanshi]

Hi,


> > 1) kf5 branch contains some commits which have open issues on
> > reviewboard, namely:
> >
> > https://git.reviewboard.kde.org/r/124305/
> > https://git.reviewboard.kde.org/r/124181/
> > https://git.reviewboard.kde.org/r/124349/
> >
>

I will look into these changes and fix whatever necessary


> > 2) kf5 branch contains modification of source code of external
> > bundled libiris library in kopete/protocols/jabber/libiris/. See
> > also file: protocols/jabber/libiris/README_BEFORE_COMMITTING
> >
> > $ git diff origin/master..origin/kf5 -- protocols/jabber/libiris/
> >
> > Due to synchronization with upstream, we either need to drop these
> > changes or send them to upstream.
>

These changes can be dropped and most of the changes are of using nullptr
instead of 0. I will look into this more and give an update to what will be
affected if it is dropped.


>
> And 3)
>
> I'm not sure if commit cb97ce4e61baa44d9f401b8853982355ef6e684f
> (Gentle quitting Kopete application) is correct.
>
> It just looks like a workaround some bug in system try area.


This commit looks okay to me as nothing much is changed in terms of
behavior. In this commit an attempt to close all the windows is made when
we are able to close all window other wise return from the function. In
normal conditions returning in the middle of for loop does not cause any
problem but I will try to verify the same for the application.
I have linked it below to make sure if this is the same commit we are
talking about.
Link :
https://cgit.kde.org/kopete.git/commit/?h=kf5=cb97ce4e61baa44d9f401b8853982355ef6e684f

Regards,
Vijay Krishnavanshi

Re: Kopete & KF5 switch

[Pali Rohár]

On Saturday 05 August 2017 14:58:20 Pali Rohár wrote:
> Hi!
> 
> Now I'm looking at Kopete kf5 git branch and thinking if now it is
> time to switch master branch to kf5 code.
> 
> IIRC KDE Applications 17.08 is the last version which can be based on
> KDE4 code base.
> 
> Before switching I see there two issues:
> 
> 1) kf5 branch contains some commits which have open issues on
> reviewboard, namely:
> 
> https://git.reviewboard.kde.org/r/124305/
> https://git.reviewboard.kde.org/r/124181/
> https://git.reviewboard.kde.org/r/124349/
> 
> 2) kf5 branch contains modification of source code of external
> bundled libiris library in kopete/protocols/jabber/libiris/. See
> also file: protocols/jabber/libiris/README_BEFORE_COMMITTING
> 
> $ git diff origin/master..origin/kf5 -- protocols/jabber/libiris/
> 
> Due to synchronization with upstream, we either need to drop these
> changes or send them to upstream.

And 3)

I'm not sure if commit cb97ce4e61baa44d9f401b8853982355ef6e684f
(Gentle quitting Kopete application) is correct.

It just looks like a workaround some bug in system try area...

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.