-
Notifications
You must be signed in to change notification settings - Fork 756
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
Reformats project based on editorconfig #5953
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small remark about a change in VectorBaseActivity.
Else LGTM, we should not wait too much before merging this PR.
@@ -83,7 +83,8 @@ class ExportEncryptionTest { | |||
@Test | |||
fun checkExportDecrypt1() { | |||
val password = "password" | |||
val input = "-----BEGIN MEGOLM SESSION DATA-----\nAXNhbHRzYWx0c2FsdHNhbHSIiIiIiIiIiIiIiIiIiIiIAAAACmIRUW2OjZ3L2l6j9h0lHlV3M2dx\n" + "cissyYBxjsfsAndErh065A8=\n-----END MEGOLM SESSION DATA-----" | |||
val input = | |||
"-----BEGIN MEGOLM SESSION DATA-----\nAXNhbHRzYWx0c2FsdHNhbHSIiIiIiIiIiIiIiIiIiIiIAAAACmIRUW2OjZ3L2l6j9h0lHlV3M2dx\n" + "cissyYBxjsfsAndErh065A8=\n-----END MEGOLM SESSION DATA-----" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not ideal but ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be because of long line wrapping (even though it still exceeds like this). Updated to put it on the same line anyway
) | ||
is GlobalError.CertificateError -> | ||
consentNotGivenHelper.displayDialog(globalError.consentUri, | ||
activeSessionHolder.getActiveSession().sessionParams.homeServerHost ?: "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this change in the "revert" commit is not expected. Can you double check that this file is properly formatted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again caused by long line wrapping. Extracted it out into a private function to make it work with formatting and just generally look cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not clear: there should be a line break after (
, and another one before )
* limitations under the License. | ||
*/ | ||
|
||
package org.matrix.android.sdk.api.auth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasn't expecting such a large diff for this file, I'm guessing the line separator character has been switched to the project defined one 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It must be so!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! with this as our baseline we can start to make adjustments 💯
Type of change
Content
Reformats project based on editorconfig
Motivation and context
The project was previously reformatted using AS settings (from preferences) that mimiced our current editorconfig. The reason for this approach at the time was to reduce the initial impact of the reformat.
After doing this, we encountered some annoyances that when we reformat the code in our own PRs, we incur additional formatting changes. This PR reformats the project once more based on the current editorconfig to finalise the formatting with the current code style and so that we don't incur such annoyances.
For context, we did discuss changing
ij_kotlin_code_style_defaults
toKOTLIN_OLD_DEFAULTS
but we know that we are going to change that in the future anyway so we decided we might as well do it now.Screenshots / GIFs
N/A
Tests
Smoke test app still works. No changes should incur as changes are purely code formatting
Tested devices
Checklist