Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Week number will be changed automatic now #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jacobtey
Copy link

@Jacobtey Jacobtey commented Jun 6, 2016

No description provided.

appName: PlasmaCalendar.Calendar.calendarBackend;
}
Timer {
interval: 500
Copy link
Owner

@anselmolsm anselmolsm Jun 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the best solution. Consider this, it is a timer triggered every 500 ms to update a value that changes once a week, i.e. the timer is triggered twice per second, 2x60=120 per minute, 120*60=7200 per hour ... 1209600 per week.

I would go for a different solution. calendarBackend exposes this C++ API: https://api.kde.org/frameworks/plasma-framework/html/calendar_8h_source.html . currentWeek is not a QPROPERTY, it is just a QML invokable, so we can't rely on something like a currentWeekChanged signal. However, there are other signals that could help us to update the week number in a more effective way.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you fixed that?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got no time for this, too many life changes since #4 was filled. I left this PR open with hopes to get an update from the author, and that did not happen. If you manage to propose a better patch, please do so.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @anselmolsm. First of all, thank you for this plasmoid. I just started using it and I find it very handy on my work laptop.

It would be very nice if this bug was fixed. How about switching the delay to 1 update/minute? Surely this is nothing on modern computers.

I think it would be better to merge this and have the plasmoid update in a suboptimal way than let it show last week's number on a Monday afternoon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants