> On July 11, 2014, 2:48 p.m., Sebastian Kügler wrote: > > It would be easier to review, if you made separate patches for the renaming > > and the actual changes in the code that juggle around the objects. > > Martin Klapetek wrote: > I did the renaming in the middle of this as I got annoyed by the > confusing names. > > To sum it up - all Rectangles are replaced by one Rectangle and two > Repeaters at the top in DaysCalendar.qml, the rest is just renaming and > sanitizing. > > Sebastian Kügler wrote: > I understand all that, it just doesn't change the fact that now, I have > to look at every changeset first, if it's just renaming or a real code > change. You want others to verify that everything's correct, because if not, > you can just commit it without review and then wait for complaints. We've > decided that that is not how we want to work together. (There's already one > thing gone unnoticed to you, so I think a good review is warranted here, > especially since, in the past, you have suggested changes which are > detrimental to the overall design, suggesting to me that you haven't fully > internalized what the important points of the design are, or that you are > making different calls than I would do. > > In the end, if changes aren't self-contained, you're accepting a > trade-off between your time and the reviewer's.
That was phrased overly negatively, there's no need for that. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119232/#review62142 ----------------------------------------------------------- On July 11, 2014, 2:35 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119232/ > ----------------------------------------------------------- > > (Updated July 11, 2014, 2:35 p.m.) > > > Review request for Plasma. > > > Repository: plasma-framework > > > Description > ------- > > Currently the grid itself is composed of 88 rectangles that draw all the > lines in a way that two big rect draws the whole two topmost horizontal and > leftmost vertical border lines and then each day rectangle is drawing small > bottom and right rect. > > This patch reduces it to 13 rects only where one rect draws the whole frame > around the grid and then 1px wide/high rects draw the inner lines. Results in > much cleaner & simple code. > > Plus there's a small refactor on the id names so it makes more sense. > > This does not require any additional changes in the applets. > > > Diffs > ----- > > src/declarativeimports/calendar/qml/MonthMenu.qml 5209816 > src/declarativeimports/calendar/qml/MonthView.qml ba3fe95 > src/declarativeimports/calendar/qml/CalendarToolbar.qml cd28702 > src/declarativeimports/calendar/qml/DayDelegate.qml 11f0afa > src/declarativeimports/calendar/qml/DaysCalendar.qml ae9df01 > > Diff: https://git.reviewboard.kde.org/r/119232/diff/ > > > Testing > ------- > > All applets using the Calendar component looks exactly the same and work > perfectly fine with no changes in the applets. > > > File Attachments > ---------------- > > digital-clock before/after > > https://git.reviewboard.kde.org/media/uploaded/files/2014/07/11/9a4f02eb-b06c-4d13-8ea5-94276659fba8__plasma_cal4.png > > > Thanks, > > Martin Klapetek > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
