This revision was automatically updated to reflect the committed changes.
Closed by commit R244:107a7fd1a3c0: add KAboutLicense::spdx and introduce
orLater qualification (authored by sitter).
REPOSITORY
R244 KCoreAddons
CHANGES SINCE LAST UPDATE
mpyne accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R244 KCoreAddons
BRANCH
spdx
REVISION DETAIL
https://phabricator.kde.org/D6672
To: sitter, sebas, mpyne
Cc: #frameworks
sitter updated this revision to Diff 16858.
sitter added a comment.
- do not use ctor delegation, can't use that in kf5 yet
- eliminate the "partial" private ctor, instead call the full private ctor
from the partial public ctors. this results in defaults being implemented in
the public
mpyne added a comment.
In https://phabricator.kde.org/D6672#126360, @sitter wrote:
> Ah drat. We could still get rid of the second private ctor though, by
moving the defaults from the private to the public and calling the full private
ctor:
Yes, that would be fine if you want to
sitter added a comment.
Ah drat. We could still get rid of the second private ctor though, by moving
the defaults from the private to the public and calling the full private ctor:
KAboutLicense::KAboutLicense(const KAboutData *aboutData)
: d(new Private(Unknown,
mpyne added a comment.
I like delegating constructors, but we can't use them in Frameworks yet. :(
The current compiler requirement policy is that we require a C++ compiler
supported in the last 3 Qt minor releases (see
sitter marked 2 inline comments as done.
sitter added a comment.
Oh, actually. Maybe we could/should make `KAboutLicense::KAboutLicense(const
KAboutData *aboutData)` delegate to the "full" public ctor. Then we can drop
`Private(const KAboutData *aboutData);` entirely.
Like so
sitter marked 3 inline comments as done.
sitter added inline comments.
INLINE COMMENTS
> mpyne wrote in kaboutdata.cpp:166
> Need a setter here for `_versionRestriction`
I am changing this one to delegate to the other ctor actually. Not much point
> mpyne wrote in kaboutdata.cpp:206
> We use
sitter updated this revision to Diff 16814.
sitter added a comment.
rebase
REPOSITORY
R244 KCoreAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6672?vs=16813=16814
BRANCH
spdx
REVISION DETAIL
https://phabricator.kde.org/D6672
AFFECTED FILES
sitter updated this revision to Diff 16813.
sitter marked an inline comment as done.
sitter added a comment.
- change "partial" Private ctor to delegate to "full" ctor so we don't have
repetitive init lists
- fix Private cctor to copy version restriction properly
- adjust copy test to
mpyne requested changes to this revision.
mpyne added a comment.
This revision now requires changes to proceed.
Still a couple of things:
1. The included new autotest fails for me, because of using `OrLaterVersion`
as a default version restriction. `OnlyThisVersion` is used elsewhere as
sitter updated this revision to Diff 16694.
sitter added a comment.
BC constructor addition
+ change to enum for orlater as suggested
+ switch param orders to take license, then restriction, then kaboutdata
kaboutdata looks and feels idiomatically like a qobject parent which also is
mpyne requested changes to this revision.
mpyne added a comment.
This revision now requires changes to proceed.
AFAICT we do need to maintain BIC here even for private ctors. The inline
comment has more detail. Other than that and with using a flag enum instead of
a `bool` I like the patch
sitter added a comment.
In https://phabricator.kde.org/D6672#124995, @sitter wrote:
> For convenience reasons it may actually be prudent to expose both `spdxID`
and `orLater` publicly. Case in point right now appstream license information
seem to be semi-expressions... they have the
sitter added a reviewer: mpyne.
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D6672
To: sitter, sebas, mpyne
Cc: #frameworks
sitter updated this revision to Diff 16634.
sitter added a comment.
add missing since tag
REPOSITORY
R244 KCoreAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D6672?vs=16633=16634
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D6672
AFFECTED FILES
sitter updated this revision to Diff 16633.
sitter added a comment.
forgot to sort out BIC
There is one theoretical BIC with the License ctor which has a new param. I
am not sure we care about this given the ctor is private. Objections welcome
but I think in the past we decied to not
sitter created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
REVISION SUMMARY
External software (e.g. appstream) uses the standardized SPDX license
identifiers. Seeing as they are specified and ours are not it is
18 matches
Mail list logo