Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. #40

已手动合并
MicheleC 4 年前 将 1 次代码提交从 issue/38/knotes 合并至 master
MicheleC 评论于 4 年前
所有者

The journal last modified time was being incorrectly marked as updated when setting the color of the note. The color was set to the same values previously read from the configuration .ics file when loading a note at the launch of KNotes application.

This PR makes sure the journal is updated only is the colors are really changed, not at each call of setColor().

The journal last modified time was being incorrectly marked as updated when setting the color of the note. The color was set to the same values previously read from the configuration .ics file when loading a note at the launch of KNotes application.<br/> This PR makes sure the journal is updated only is the colors are really changed, not at each call of setColor().
MicheleC4 年前 添加了标签 PR/rfc
MicheleC4 年前 添加了里程碑 R14.1.0 release
MicheleC 评论于 4 年前
发布者
所有者

Ups... looks like Kontact is still messing up things. More work will be required to fix that. Anyhow if you can, please test the KNotes related part for the time being. I will work on the Kontact part tomorrow.

Ups... looks like Kontact is still messing up things. More work will be required to fix that. Anyhow if you can, please test the KNotes related part for the time being. I will work on the Kontact part tomorrow.
MicheleC4 年前 修改标题 Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes.WIP: Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes.
deloptes 评论于 4 年前
协作者

Wouldn't it be better to rework knotes and kpart?

It might be more work, but at the end would be cleaner and I'm sure this problem will disappear.

I don't know if it is possible or how it would be possible, but it looks like it would require a backend to handle the notes and fromtend part - the application and another one - kpart_knotes.

It looks like to me that both are now somehow detached.

Wouldn't it be better to rework knotes and kpart? It might be more work, but at the end would be cleaner and I'm sure this problem will disappear. I don't know if it is possible or how it would be possible, but it looks like it would require a backend to handle the notes and fromtend part - the application and another one - kpart_knotes. It looks like to me that both are now somehow detached.
MicheleC 评论于 4 年前
发布者
所有者

Hi Emanoil,

knotes_part is the KPart that can be embedded into other applications, like Kontact for example. KNotes is the original application, which I assume was developed before the Kpart thing.

Short term, I will work in fixing the kpart as well, I expect to be something similar to the first commit.

Long term solution would be to remove duplication of code and have KNotes to use the KPart as well, similar to what Kontact does. We can look into doing this after R14.1.0, to avoid further delay on the release of that. Also long term we need to work on a fix to get Knotes and Kontact sync together, to avoid changes in one app to be lost into the other (there is already issue #16 for that)

Hi Emanoil,<br/> knotes_part is the KPart that can be embedded into other applications, like Kontact for example. KNotes is the original application, which I assume was developed before the Kpart thing. Short term, I will work in fixing the kpart as well, I expect to be something similar to the first commit. Long term solution would be to remove duplication of code and have KNotes to use the KPart as well, similar to what Kontact does. We can look into doing this after R14.1.0, to avoid further delay on the release of that. Also long term we need to work on a fix to get Knotes and Kontact sync together, to avoid changes in one app to be lost into the other (there is already issue #16 for that)
MicheleC 评论于 4 年前
发布者
所有者

PR updated with the fix for Kontact as well (just one extra line of code 😄).

Please test and let me know if it works for you too.

PR updated with the fix for Kontact as well (just one extra line of code :smile:).<br/> Please test and let me know if it works for you too.
MicheleC4 年前 修改标题 WIP: Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes.Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes.
deloptes 评论于 4 年前
协作者

Hi Michele, regarding your previous post, it sounds like a very good plan.

Regarding this patch, I will try to test in the coming days, but at the end you again emit sigDataChanged whenever color was changed. I understand it is part of the note, but I am not sure it should be, however I would accept it. From first sight it looks promising.

regards

Hi Michele, regarding your previous post, it sounds like a very good plan. Regarding this patch, I will try to test in the coming days, but at the end you again emit sigDataChanged whenever color was changed. I understand it is part of the note, but I am not sure it should be, however I would accept it. From first sight it looks promising. regards
MicheleC 评论于 4 年前
发布者
所有者

Hi Emanoil,

test here looks good so far, so I don't think emitting sigDataChanged is a problem. The problem was with the incorrect update of the journal entry, not the note itself.

Anyhow you may have more or different test cases from me, so you may run in different scenarios. If you testing is good as well, I will merge.

Hi Emanoil,<br/> test here looks good so far, so I don't think emitting sigDataChanged is a problem. The problem was with the incorrect update of the journal entry, not the note itself.<br/> Anyhow you may have more or different test cases from me, so you may run in different scenarios. If you testing is good as well, I will merge.
deloptes 评论于 4 年前
协作者

Hi Michele,

yes, tests were good.

Hi Michele, yes, tests were good.
SlavekB 评论于 4 年前
所有者

This looks good and at first glance it seems to me that it can also be used for r14.0.x branch.

This looks good and at first glance it seems to me that it can also be used for r14.0.x branch.
MicheleC4 年前 关闭此合并请求
MicheleC4 年前 删除了分支 issue/38/knotes
MicheleC4 年前 删除了标签 PR/rfc
MicheleC 4 年前 修改了里程碑从 R14.1.0 releaseR14.0.8 release
MicheleC 评论于 4 年前
发布者
所有者

Thanks Emanoil and Slavek. Merged and backported to R14.0.8 too.

Thanks Emanoil and Slavek. Merged and backported to R14.0.8 too.
合并请求已被手动合并为 d1a4daaee6
登录 并参与到对话中。
无审核者
未选择里程碑
未指派成员
3 名参与者
通知
到期时间

未设置到期时间。

依赖工单

没有设置依赖项。

参考:TDE/tdepim#40
正在加载...
这个人很懒,什么都没留下。