Skip to content
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

Formats project with new code style #5805

Merged
merged 4 commits into from
Apr 21, 2022
Merged

Conversation

ericdecanini
Copy link
Contributor

@ericdecanini ericdecanini commented Apr 20, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Autoformats project

Motivation and context

Aligns project with new editorconfig standards

This is to prepare for incremental code style changes, and should be the only PR in the code style subproject with a massive diff, thus the greatest challenge.

I tested conflicts for this by merging this branch into the Tchap fork and this seemed to introduce only a few more easy-to-resolve conflicts than there would be when merging with develop

See also #5644

Screenshots / GIFs

N/A

Tests

  • Checkout Tchap fork
  • Add the original element repo as upstream and fetch all the branches
  • Merge this branch with Tchap develop

You will see that there are a lot of conflicts, but most of these conflicts come from changes that are on our repo's develop and not the changes on this branch specifically. If you repeat the steps above but instead merge upstream/develop into origin/develop, you will see that most of those conflicts are still present and this branch adds only few.

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 10

Checklist

@github-actions
Copy link

github-actions bot commented Apr 20, 2022

Unit Test Results

114 files  ±0  114 suites  ±0   1m 36s ⏱️ +13s
202 tests ±0  202 ✔️ ±0  0 💤 ±0  0 ±0 
678 runs  ±0  678 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit a5eca12. ± Comparison against base commit 7f3e72b.

♻️ This comment has been updated with latest results.

if (isTracking) {
val translationY = event.y - startY
swipeView.translationY = translationY
onSwipeViewMove(translationY, translationLimit)
}
return true
}
else -> {
else -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand why we have so many changes.
The plan was to change the rules one by one.
So:

  • create the .editorconfig with the current rules, eventually format the whole code base to fix existing issues.
  • Change the rules one by one, in .editorconfig and then format, so for instance, change the rule for arrow alignment and see in the PR only changes about the arrows.

WDYT?

Copy link
Contributor Author

@ericdecanini ericdecanini Apr 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format here is actually without any rule changes. The problem we're facing is the app doesn't consistently abide by our currently set rules and breaks many basic lint rules in lots of places.

Update: By not using editor config (i.e. using android studio format settings), I was able to massively reduce the diff. This is the smallest autoformat I can do on the whole project so that the code style (reflecting the current) is consistent all around

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current rule is to align the arrows in a when block vertically and here it is changing, so I do not understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Updated the PR fixing this. This file no longer has any diffs)

@ericdecanini ericdecanini force-pushed the task/eric/format-project branch from 03aa7f7 to de899bb Compare April 21, 2022 09:50
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, with this changes

@ericdecanini ericdecanini merged commit c21ec98 into develop Apr 21, 2022
@ericdecanini ericdecanini deleted the task/eric/format-project branch April 21, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants