-
Notifications
You must be signed in to change notification settings - Fork 92
Code refactor #495
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
base: main
Are you sure you want to change the base?
Code refactor #495
Conversation
Thank you for the pull request! ❤️The Scribe-Android team will do our best to address your contribution as soon as we can. If you're not already a member of our public Matrix community, please consider joining! We'd suggest that you use the Element client as well as Element X for a mobile app, and definitely join the |
Maintainer ChecklistThe following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)
|
7f5edf3
to
3e6ef71
Compare
@andrewtavis Sorry for the delay. I completed this a few weeks ago and thought I made the PR but I realized I didn't. The GeneralKeyboard file still has ~1700 lines and I am sure it can be further improved. I'd like to know your suggestions on this. Thank you! |
3e6ef71
to
c1807d7
Compare
Thanks so much for the PR, @Nishthajain7! We'll get to the review in the coming days :) :) |
Would you be able to check out the failing instrumentation tests though? |
Can I also take a look into why the instrumentation tests are failing? @andrewtavis |
Sure thing, @Akshaykomar890! Would appreciate your help with the review :) |
I fixed the failing instrumentation test! 🎉 Issue: The Fix: Nishthajain7#1 Tested locally - test passes now @Nishthajain7 Once you merge my PR and update this branch, CI should pass! Let me know if you need anything 👍 |
Thanks so much for the great collaboration, @Akshaykomar890! @Nishthajain7, would you want to merge in Nishthajain7#1 and then we can bring the changes here? From there we can work to bring this PR in? 😊 |
Thanks, @andrewtavis 😊 That sounds great. Once @Nishthajain7 merges the fix, I’ll help verify that everything’s running fine so we can move this PR forward smoothly. |
(cherry picked from commit f0dc571)
I have added the commit here @Akshaykomar890 |
I will try fixing the conflicts |
73de011
to
0a36e03
Compare
I will try fixing the errors later today |
Contributor checklist
./gradlew lintKotlin detekt test
command as directed in the testing section of the contributing guideDescription
Related issue