Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

Balance row enhancement, attempt #2 #2847

Merged
merged 4 commits into from
Dec 21, 2023
Merged

Conversation

nvllz
Copy link
Contributor

@nvllz nvllz commented Dec 21, 2023

Idk... made another attempt at this thing. If this one doesn't work, I'll prooly give up on it, as making it made me refactor a lot of other things to not get weird warnings. I hope it passes these tests so we can have these lines look like they should.

#2821

…, fixed some parameters order and minor errors
@ILIYANGERMANOV
Copy link
Collaborator

You can add Detekt baseline to fix the Detekt errors caused by the legacy code

@nvllz
Copy link
Contributor Author

nvllz commented Dec 21, 2023

Okay man, now we just have to fix this Detekt shit. Well... I really don't know what you mean as I'm not familiar with these plugins and checks. I'm getting these errors and figured out I need to remove some empty lines, but I can't get rid of modifier parameters as I get problems in Android Studio saying "this function needs it". I need your help and as you can see we are very close to making it a thing before 2024 :D

Property 'style>OptionalWhenBraces' is deprecated. Same functionality is implemented in BracesOnWhenStatements.
/home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:95:44: The function EditBottomSheet(initialTransactionId: UUID?, type: TransactionType, accounts: List<Account>, selectedAccount: Account?, toAccount: Account?, amount: Double, currency: String, amountModalShown: Boolean, setAmountModalShown: (Boolean) -> Unit, ActionButton: @Composable () -> Unit, onAmountChanged: (Double) -> Unit, onSelectedAccountChanged: (Account) -> Unit, onToAccountChanged: (Account) -> Unit, onAddNewAccount: () -> Unit, modifier: Modifier, convertedAmount: Double?, convertedAmountCurrencyCode: String?) has too many parameters. The current threshold is set to 12. [LongParameterList]
/home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:95:29: The function EditBottomSheet is too long (159). The maximum length is 150. [LongMethod]

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':detekt'.
> Analysis failed with 10 weighted issues.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org./

BUILD FAILED in 1m 17s
/home/runner/work/ivy-wallet/ivy-wallet/screen-categories/src/main/java/com/ivy/categories/CategoriesScreen.kt:256:2: Needless blank line(s) [NoConsecutiveBlankLines]
/home/runner/work/ivy-wallet/ivy-wallet/screen-categories/src/main/java/com/ivy/categories/CategoriesScreen.kt:471:2: Needless blank line(s) [NoConsecutiveBlankLines]
/home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:649:2: Needless blank line(s) [NoConsecutiveBlankLines]
/home/runner/work/ivy-wallet/ivy-wallet/screen-categories/src/main/java/com/ivy/categories/CategoriesScreen.kt:425:5: Function parameter `modifier` is unused. [UnusedParameter]
/home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/legacy/ui/theme/modal/IvyModalDomainComponents.kt:28:5: Function parameter `modifier` is unused. [UnusedParameter]
/home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:110:5: Function parameter `modifier` is unused. [UnusedParameter]
/home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:672:13: Private property `spacerInteger` is unused. [UnusedPrivateProperty]
/home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:673:13: Private property `currencyPaddingTop` is unused. [UnusedPrivateProperty]


> Task :detekt FAILED
10 actionable tasks: 10 executed
Configuration cache entry stored.

@ILIYANGERMANOV
Copy link
Collaborator

@nvllz does the Detekt baseline script work for you? If you generate a baseline Detekt won't complain. Sorry, I can't help much - I no longer work on the project and just help folks merge their PRs. But in order to merge them we need CI ✅ Try ./scripts/detektBaseline.sh or whatever the script it called.

If the script isn't working for some reason, it's way easier just fixing these consecutive lines.

./gradlew detekt

to run ut locally and fix errors fast and easy

@nvllz
Copy link
Contributor Author

nvllz commented Dec 21, 2023

The first command executes without any message, and the second throws me this error:

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':buildSrc:compileKotlin'.
> Unknown Kotlin JVM target: 21

I'm so confused. I can build the app without any errors and this detekt thing shows some problem messages that I can't understand. Like: /home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:95:29: The function EditBottomSheet is too long (159). The maximum length is 150. [LongMethod]... but is it really my problem? I didn't mess with those things. Is there any way to just skip this, at least once? Please respect the thing I spent so much time on :/

@ILIYANGERMANOV
Copy link
Collaborator

The first command executes without any message, and the second throws me this error:

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':buildSrc:compileKotlin'.
> Unknown Kotlin JVM target: 21

I'm so confused. I can build the app without any errors and this detekt thing shows some problem messages that I can't understand. Like: /home/runner/work/ivy-wallet/ivy-wallet/temp-legacy-code/src/main/java/com/ivy/legacy/ui/component/edit/core/EditBottomSheet.kt:95:29: The function EditBottomSheet is too long (159). The maximum length is 150. [LongMethod]... but is it really my problem? I didn't mess with those things. Is there any way to just skip this, at least once? Please respect the thing I spent so much time on :/

Yes, you can skip it (in fact you should) and the way to do that is to generate Detekt baseline. Sorry, for the bad experience but I can't merge your PR with broken CI because no one after you won't be able to merge. CI must be green ✅

Ways to fix:

  1. Add Detekt baseline
  2. Edit the LongMethod rule to allow 160

For both of this the free ChatGPT should be able to help

@ILIYANGERMANOV
Copy link
Collaborator

Did you try:

gradlew detektBaseline

@nvllz
Copy link
Contributor Author

nvllz commented Dec 21, 2023

Okay, I somehow made it all green manually. Sorry for my anger, but I was so confused with these bugs. It's kind of black magic for me, but I wanted this improvement so much. I hope it's okay.

Now please take your time to review the changes, merge this PR and consider a new release, as I can see many improvements and fixes since the last update. I hope you at least plan on releasing new versions from time to time.

Thanks, Iliyan!

@ILIYANGERMANOV ILIYANGERMANOV merged commit 2421ba8 into Ivy-Apps:main Dec 21, 2023
6 checks passed
@ILIYANGERMANOV
Copy link
Collaborator

No worries, @nvllz! Thank you for your patience :) Merged 🚀

@ILIYANGERMANOV
Copy link
Collaborator

Okay, I somehow made it all green manually. Sorry for my anger, but I was so confused with these bugs. It's kind of black magic for me, but I wanted this improvement so much. I hope it's okay.

Now please take your time to review the changes, merge this PR and consider a new release, as I can see many improvements and fixes since the last update. I hope you at least plan on releasing new versions from time to time.

Thanks, Iliyan!

About releasing, the community need to test everything really really well cuz if we release sth broken, I won't have time to fix it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants