Details
- Reviewers
- None
Diff Detail
- Branch
- dev/mimetreeinterface
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 4170 Build 4171: arc lint + arc unit
Event Timeline
framework/domain/mimetreeparser/interface.h | ||
---|---|---|
59 | Consider using a typedef: Part::Ptr for all std::shared_ptr usages | |
76 | parent() | |
92 | Why do we have it if we don't use it? | |
98 | *disposition | |
99 | Isn't this a string? | |
104 | void overrideCharset(const QByteArray &charset) ? I assume the part needs the override charset to be able to correctly return content()? | |
138 | You can't overload by return type. Perhaps encodedContent()? | |
141–142 | These two should be in MimePart, they can apply to attachments as well. | |
181 | Where is CryptoBackend defined? | |
209 | Where would we find this part? | |
274 | Where's KeyTrust defined? | |
288 | Since we have a static interface, how would you react to changes? | |
314 | Can we avoid the GpgME dependency in the interface? |
framework/domain/mimetreeparser/interface.h | ||
---|---|---|
92 | just a copy of the KMime:Content enum with these comments :D | |
104 | overrideCharset is only needed if you really want to overwrite the default charset defined in the mail. The default is not to overwrite the charset. | |
138 | well the default should be that you don't need to change the charset at all, becuase the sender emailclient is configured correctly :D, thats why I dislike another parameter. But maybe: QString encodedContent(); //use default charset QString encodedContent(overwrittenCharset); | |
141–142 | muste be moved to Part, because also TextParts can be encrepted/signed | |
181 | currently it is not configured, inside mimetreeparser it is a pointer to smime/pgp backend of Gpgme | |
209 | currently only if smimeType == QLatin1String("certs-only") | |
274 | Currently nowhere, but to be more flexible, we need a Class here that can be implemented for different KeyTrust Types in their Subcalsses. | |
288 | Well for QML we anyhow need QObjects with properties, so we also can add signals, the underlaying gpgme is already async. | |
314 | The problem is that if we make to genric here, we end with a libkleo2 than is only a interface for a interface, that has only one implementation. As I already written if we need notation for mail verification, i'm not sure. And a client may need to show the full details of a signature. And these may be need to displayed differntly for every signature type. I have no good idea here how to create a nice interface. |
framework/domain/mimetreeparser/test.cpp | ||
---|---|---|
43 | Same as above? |
framework/domain/mimetreeparser/interface.h | ||
---|---|---|
92 | In that case please clean it up and map explicitly. | |
138 | Yes, you can either have default argument or do an overload. My point is if overrideCharset above is only needed for this, then I'd rather just have the parameter on this function. | |
141–142 | At which point the MimePart/Part distinction becomes questionable because the only other part is the error part. | |
181 | If it becomes part of the interface we need to know what it is. | |
209 | Why would we not just search for an attachment part of a specific mime-type? | |
274 | hmm... We need one consistent behaviour though for the UI, do you already have ideas how that interface could look like? | |
288 | No, we switched to models a while ago, so there is no requirement for QObjects at all. The current interface doesn't look like it's asynchronous, so if we really need that feature we need some sort of callback or signal. Are there other properties that change dynamically? | |
314 | The alternative is that we directly return qgpgme structures, which means you'll have to stick with that and trust in the binary compatibility promises since you're interface will inherit any problems in that. Another variant would be to completely hide anything qgpgme, and you just define everything that we need to display in your interface. |
framework/domain/mimetreeparser/interface.h | ||
---|---|---|
141–142 | No we also have TextPart "just" representing a QByteArray. To make GPGInline workable. And other structural elements like EncryptionPart and SignaturePart. So we have different types of Parts: | |
314 | void pointer - no way with that we really make the software not safger in terms of BIC and API breakage :D Because now nobody knows, that is inside this void pointer. can something like this help: class gpgmestructure; class Signature { public: str::shared_ptr<gpgmestructure> getGPGmepointer(); } and in private header: class gpgmestructure { public: GpgMe::Signature signature(); } this way we make public interface free of use of GpgMe and make it obvious to use the private headers with risk? If we have the risk of BIC/API breakage, than we need to add complete interface for the GpgMe structures. This means: class Singature class GPGSignature : public Signaute and anytime in future maybe: class PEPSignature : public Signature |
mmh this workflow does not work - if I update the branch the diff is not updated and the diff is only show for one commit.
I updated the patch:
https://phabricator.kde.org/rKUBE0ed9290279f0fa50ece3bed46813fb863a7713eb
We also want to chagne the discussion to phabricator.kde.org