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

Balance row enhancement #2821

Closed
wants to merge 4 commits into from
Closed

Balance row enhancement #2821

wants to merge 4 commits into from

Conversation

nvllz
Copy link
Contributor

@nvllz nvllz commented Nov 17, 2023

Pull Request (PR) Checklist

Please check if your pull request fulfills the following requirements:

  • The PR is submitted to the main branch.
  • I've read the Contribution Guidelines and my PR doesn't break the "Contributing Rules".
  • I've read the Architecture Guidelines.
  • I confirm that I've run the code locally and everything works as expected.
  • 🎬 I've attached a screen recoding of the changes.

What's changed?

It's more of pushing forward the #2742 PR based on my initial commit. Looks the same like on a screen recording there, but I fixed some arguments order.

Does this PR closes any GitHub Issues?

Troubleshooting CI failures

If you see any of the PR checks failing (❌) go to Actions and find it there. Or simply click "Details" next to the failed check and explore the logs to see why it has failed.

Detekt

Detekt is a static code analyzer for Kotlin that we use to enforce code readibility and good practices.

To run Detekt locally:

./gradlew detekt

If the Detekt errors are caused by a legacy code, you can suppress them using a basline.

Detekt baseline (not recommended)

./scripts/detektBaseline.sh

Lint

We use the standard Android Lint plus Slack's compose-lints as an addition to enforce proper Compose usage.

To run Lint locally:

./scripts/lint.sh

If the Lint errors are caused by a legacy code, you can suppress them using a basline.

Lint baseline (not recommended)

./scripts/lintBaseline.sh

Unit tests

If this job is failing this means that your changes break an existing unit test. You must identify the failing tests and fix your code.

To run the Unit tests locally:

./gradlew testDebugUnitTest

@nvllz
Copy link
Contributor Author

nvllz commented Nov 17, 2023

Idk.. It works well in the android emulator inside Android Studio. I don't know what do these checks mean, but there was nothing wrong. How can changing so little make it fail? Is there anyone who knows how to fix this and merge this code in order to make the balance row look more friendly? Thanks, I'm just helpless..

Copy link
Collaborator

@ILIYANGERMANOV ILIYANGERMANOV left a comment

Choose a reason for hiding this comment

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

Lint is probably failing because of baseline.
Regarding the UI can you make a screenrecording, I can't imagine how it looks without seeing it first

app/lint-baseline.xml Outdated Show resolved Hide resolved
@nvllz
Copy link
Contributor Author

nvllz commented Nov 17, 2023

It's literally the same as shown in this PR's comment, as it's based on the same code. I just had to rearrange some compose attributes (it was forced by the @deprecared line) and used other variable name for balance line. #2742

I just want it to proceed, sorry hahah. These errors made me feel like it's impossible

@ILIYANGERMANOV
Copy link
Collaborator

Fix the CI and we can merge

@nvllz
Copy link
Contributor Author

nvllz commented Nov 17, 2023

Can you help me with this thing? I don't even know what does it mean. Just edited some small piece of code like in Android Studio and idk what now. Never had such a problem with my amateur contributions :/

@ILIYANGERMANOV
Copy link
Collaborator

Can you help me with this thing? I don't even know what does it mean. Just edited some small piece of code like in Android Studio and idk what now. Never had such a problem with my amateur contributions :/

Try to build with Android Studio seems like the project isn't building at all. Also, check the CI Troubleshooting steps in the PR description

@github-actions github-actions bot added the Stale No activity, will be automatically closed soon. label Nov 21, 2023
@github-actions github-actions bot closed this Nov 22, 2023
@nvllz
Copy link
Contributor Author

nvllz commented Nov 26, 2023

Okay, @ILIYANGERMANOV. I successfully built the app and took a screen recording. It has to be a problem with these github automated things (which also keep closing my PRs and feature request tickets.. ugh) as this build works fine. Don't know what to do now to let this PR make the app.

signal-2023-11-26-15-23-26-371.mp4

@github-actions github-actions bot added Stale No activity, will be automatically closed soon. and removed Stale No activity, will be automatically closed soon. labels Nov 27, 2023
@github-actions github-actions bot closed this Dec 2, 2023
@nvllz
Copy link
Contributor Author

nvllz commented Dec 21, 2023

@ILIYANGERMANOV can you merge this? Idk what's going on right now

@ILIYANGERMANOV
Copy link
Collaborator

@ILIYANGERMANOV can you merge this? Idk what's going on right now

Hey @nvllz I can only if all CI checks are ✅ I'll also review the PR once we have CI greenlight, don't remember it. Anyway, the CI checks are mandatory

@ILIYANGERMANOV
Copy link
Collaborator

Follow the CI troubleshooting steps in the PR description

@ILIYANGERMANOV
Copy link
Collaborator

Attempt 2 was merged successfully

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale No activity, will be automatically closed soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the balance display style in Accounts tab
2 participants