-
Notifications
You must be signed in to change notification settings - Fork 197
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
Rename OmemoEnvelope's 'setIsUsedForKeyExchange()' to 'setUsedForKeyExchange()' #468
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportBase: 68.42% // Head: 68.42% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #468 +/- ##
==========================================
- Coverage 68.42% 68.42% -0.01%
==========================================
Files 279 279
Lines 24306 24305 -1
==========================================
- Hits 16631 16630 -1
Misses 7675 7675
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
there are probably other places where this is the case, can you also adjust them? |
There are the following setters:
Changing 1, 2, 3 and 7 could confuse the users because it gets a different meaning. E.g., |
I'd like to stick to the Qt conventions as much as possible. https://wiki.qt.io/API_Design_Principles#Naming_Boolean_Getters.2C_Setters.2C_and_Properties It says:
At first I agreed that having a setter called However, it's probably a good idea to use more explicit types such as enums or structs to avoid such situations. For example:
I'd suggest to:
It would be great if you could do 3. in this PR, the other points can be done later (if you don't want to do them I can do them too). :) |
84168ac
to
e614dcc
Compare
PR check list:
\since QXmpp 1.X
,QXMPP_EXPORT
-DBUILD_DOCUMENTATION=ON
)doc/doap.xml
clang-format -i src/<edited-file(s)> tests/<edited-file(s)>