Skip to content

Conversation

Rsedp8
Copy link
Contributor

@Rsedp8 Rsedp8 commented May 24, 2025

this PR addresses multiple inspection warnings in Android Studi related to #5996.
Changes include:

  KDoc fixes IDE warnings:
    Removed invalid @param and @return tags from properties (var/val).
    Replaced or corrected mismatched @param tags in functions (e.g. wrong parameter names like filePath).
    Escaped invalid [[wikilinks]] to avoid symbol resolution errors.      
    Fixed malformed or broken KDoc blocks (e.g. extra *, misspelled tags like @param nsumers, etc.).

Code consistency improvements IDE warnings:
    Replaced unused parameters in methods or documented them as (currently unused) where appropriate.
    Removed or corrected dead or commented-out code blocks referencing invalid parameters (e.g. activity, imageUri, etc.).

These changes do not alter functionality, but reduce IDE noise, and align with Kotlin/Java documentation best practices.

What changes did you make and why?
Fixed invalid or misleading documentation that caused IDE warning.
Aligned all documentation with actual function signatures and property names to fix IDE warning.

No UI or logic changes were introduced, only documentation and declaration adjustments.

Screenshots (for UI changes only)
Not applicable – this PR does not affect UI.

Rsedp8 added 30 commits May 24, 2025 16:38
Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Would you mind fixing this? Thanks! :-)

> Task :app:kaptBetaDebugAndroidTestKotlin FAILED
/home/runner/work/apps-android-commons/apps-android-commons/app/build/tmp/kapt3/stubs/betaDebugAndroidTest/fr/free/nrw/commons/navtab/MoreBottomSheetLoggedOutFragmentUnitTests.java:4: error: incompatible types: NonExistentClass cannot be converted to Annotation
@error.NonExistentClass()
      ^

@Rsedp8
Copy link
Contributor Author

Rsedp8 commented May 26, 2025

Would you mind fixing this? Thanks! :-)

> Task :app:kaptBetaDebugAndroidTestKotlin FAILED
/home/runner/work/apps-android-commons/apps-android-commons/app/build/tmp/kapt3/stubs/betaDebugAndroidTest/fr/free/nrw/commons/navtab/MoreBottomSheetLoggedOutFragmentUnitTests.java:4: error: incompatible types: NonExistentClass cannot be converted to Annotation
@error.NonExistentClass()
      ^

ContributionsListFragmentUnitTests and MoreBottomSheetLoggedOutFragmentUnitTests class are located in androidTest but uses Robolectric (@LooperMode, Shadows), which only works in test if i believe what i read. This caused kapt to generate @error.NonExistentClass. Moving the test to the correct test/ source set should fix the issue ?

@nicolas-raoul
Copy link
Member

Would you mind trying?
Thanks a lot! 🙂

@Rsedp8
Copy link
Contributor Author

Rsedp8 commented May 26, 2025

sure i will try

@Rsedp8
Copy link
Contributor Author

Rsedp8 commented May 26, 2025

@nicolas-raoul This is currently above my level of java programming haha. I'm still learning, never used robolectric , i need to learn how to adapt tests like this one to work outside of androidTest/. I’ll submit a proper pull request with a working solution as soon as I’ve learned more about Robolectric and test setup.

@Rsedp8
Copy link
Contributor Author

Rsedp8 commented May 26, 2025

@nicolas-raoul can you please open an issue about it and assign it to me ? or should i open it ?

@nicolas-raoul
Copy link
Member

nicolas-raoul commented May 27, 2025

If this is needed to fix the build of this pull request, then no need to create a separate issue. 🙂

@Rsedp8
Copy link
Contributor Author

Rsedp8 commented May 27, 2025

If this is needed to fix the build of this pull request, then no need to create a separate issue. 🙂

oh i see these is related to the warning i fix ? i will investigate futher for now i dont understand what is making the build fail from the fixes made

Copy link

✅ Generated APK variants!

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Somehow the unit tests seem to fail on Continuous Integration, then succeed a few minutes later.
I just tried on my machine and they pass successfully.
The changes are all good and I don't think they would trigger such error, so I will now merge.
Sorry for the delay, and thanks a lot for your contribution! :-)

@nicolas-raoul nicolas-raoul merged commit cfc2cfc into commons-app:main May 29, 2025
1 check passed
@Rsedp8
Copy link
Contributor Author

Rsedp8 commented May 29, 2025

Thanks for your help and explanation with the CI issue. It was great to contribute, even if it was mainly documentation. I'm definitely going to dig deeper into why the tests fail sporadically. It was a pleasure to lend a hand on a project I enjoy using as a user.

@nicolas-raoul
Copy link
Member

Feel free to take any other issue if you have time, anything is helpful. 🙂

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