rkflx added a comment.
In https://phabricator.kde.org/D7164#156062, @dfaure wrote:
> Looks like the CI issue is gone?
Thanks for caring, David. Maybe I could have made this more clear, but work
regarding the CI failure is tracked in https://phabricator.kde.org/T6982, were
I
dfaure added a comment.
Looks like the CI issue is gone?
https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20XenialQt5.7/
looks green to me (well, blue, the jenkins people got daltonism issues lately).
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
rkflx added a comment.
Thank you so much for your help until now, Dominik! Unfortunately, there's
one more thing: The CI does not like `testChrome` :( Could you tell me how the
workflow is now?
- Should this be discussed here? New review request? Just commit?
- What's the timeframe
This revision was automatically updated to reflect the committed changes.
Closed by commit R236:69e9e2ca2230: KSqueezedTextLabel: Respect indent, margin
and frame width (authored by rkflx).
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.
I think this is good enough, especially since according to your research the
previous code also exists in certain Qt releases also as LGPLv2.
REPOSITORY
R236 KWidgetsAddons
BRANCH
rkflx updated this revision to Diff 19345.
rkflx edited the summary of this revision.
rkflx added a comment.
Spent some time trying to solve this properly, but it's difficult.
We could get the Qt code as LGPLv2.1 from an older release, and
`relicensecheck.pl` for
dhaumann added a comment.
Looks good to me, except for the licensing issue. I am not really an expert
here, but LGPLv3 to LGPLv2 only is problematic, as far as I know. Do you see a
good solution?
INLINE COMMENTS
> ksqueezedtextlabel.cpp:128
> +{
> +// copied and adapted from
rkflx updated this revision to Diff 19332.
rkflx edited the summary of this revision.
rkflx added a comment.
- remove custom setters for lineWidth, margin and indent and do not re-mark
them as properties, i.e. no more uncertainty about BIC (not needed anymore
after fixing autotest in
rkflx planned changes to this revision.
rkflx added a comment.
Thanks for looking at this so quickly, Christoph! I believe you inspired me
to find a good solution. I'll submit updates over the weekend.
If you are interested in the backstory, read on:
> I probably miss the big
cfeck added a comment.
QLabel sends out a QEvent::ContentsRectChange event when the margins change
or the frame style causes a size change. Could this be catched (using an event
filter) instead of reimplementing the properties?
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
cfeck added a comment.
I probably miss the big picture here. Why are these attributes needed? The
lineWidth attribute, for example, looks like a thing from the past, where you
could control the thickness of frames (CDE vs. Motif style).
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
rkflx added a reviewer: cfeck.
rkflx added a comment.
@cfeck Friendly ping :)
Please let me know if you have any questions or someone else should review
this. We really don't want to BIC-break a Tier 1 framework…
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
dhaumann added a subscriber: cfeck.
dhaumann added a comment.
To me this looks good.
It seems Q_PROPERTYs can be overridden (at least, this follows imho from "The
presence of the FINAL attribute indicates that the property will not be
overridden by a derived class" from
rkflx updated this revision to Diff 17972.
rkflx added a comment.
Same wording for reimplementation warning as in
https://phabricator.kde.org/D7161.
Make indent, margin and lineWidth Q_PROPERTIES.
Add link to phabricator discussion as KF6 TODO (not sure if we can reach a
conclusion
rkflx added a comment.
Some thoughts, inconclusive though:
Another idea: Sometimes additional functionality for non-virtual functions is
provided by hooking into the changeEvent(). However, for our use case we won't
get such events from QLabel.
Things we might want to consider when
dhaumann added a comment.
This patch adds four functions that all "reimplement" functions that exist in
base classes.
However, these functions are not virtual. As such, adding the functions is
probably fine (BC), but essentially the correct behavior now depends on calling
the correct
16 matches
Mail list logo
