commons-app / apps-android-commons

The Wikimedia Commons Android app allows users to upload pictures from their Android phone/tablet to Wikimedia Commons
https://commons-app.github.io/
Apache License 2.0
1.02k stars 1.23k forks source link

Plans for 2018 - WMCZ proposal #865

Closed misaochan closed 6 years ago

misaochan commented 7 years ago

As some of you may know, Wikimedia Czech Republic has kindly offered to include us in their grant proposal for 2018 funding. :) We will need to come up with a list of improvements that we intend to implement in 2018. @VojtechDostal has informed me that their proposal is due on 1 Oct 2017, so we will need to "finalize" the list by the end of September ideally.

Feedback on the proposed improvements, as well as suggestions for new improvements, are most welcome. The scope will be for 10 months of development (most likely by the same team as the renewal proposal), starting March 2018 and ending Dec 2018. Development time must also include time needed for fixing bugs and issues with implementation, as well as for mandatory reporting.

Legend: [HP] - High-priority improvements [Optional] - we are currently undecided on whether we should include these

User-focused improvements

Further Nearby enhancements

Others

Finally

Technical improvements

No Library Needed Library Needed
[HP] Move to JSON based WIkimedia API (formerly, 'use Retrofit'). We already include OkHttp and GSON in the build. [HP] Use Glide library instead of asynctask load of images. Glide already developed for loading images, and its support different paths for imageview (URI, URL, file path at SD card and etc). Simple use, simple result
[HP] For fragments, use newInstance pattern instead of creating new objects. Fragment newInstance() pattern prevent memory leaks [HP] Add dagger implementations to project. Dagger is dependency injection library which will help to remove many initializations of some utils/loaders/api workers and etc. For example in current project it will help to initialize SharedPreferences client once for using in many parts of app. (the runtime for Dagger is tiny, it's mostly compile-time annotation processing)
[HP] Most common sizes must be moved to dimens.xml and separated by screen-sizes folders. Too much values are hardcoded in layouts Move layouts to ConstraintLayout where its possible, it much smoother and easy to operate. (but it's a Google support library)
[HP] Move icons to SVG format instead of PNG, which will decrease APK size and improve icon multi-screen support
User shrinkResources = true in build.gradle to remove unused resources from APK file on release.
Add rxAndroid to project - it will speedup application, remove logic from main thread and provide huge possibilities make multithreading apps (already underway)
[Optional] Android data binding for activities, fragments, viewholders. Android data binding - powerful technique which simplify view code, and let you add some boilerplate logic to XML instead of some operations directly in activity, fragment, viewholder. Renderers can be replaced by Android data binding implementations
[HP] Address the very high cyclomatic complexity we are seeing from Codacy by breaking up Activities & Fragments according to one of the well-respected architectural patterns (MVP / MVVM / etc)
VojtechDostal commented 7 years ago

Hi all and thanks @misaochan for starting this. I am also posting @tised 's suggestions on the technical side of the app which you might or might not find useful - just wanted you to have them at your disposal:

psh commented 7 years ago

On the technical topics, generally a "yes" to all with the following notes:

Codacy is reporting some very high cyclomatic complexity numbers for classes like ShareActivity and MediaDetailsFragment. I think we should take a look at breaking up some of these big / complex classes to make the code more maintainable. Some (architectural) separation between View, Model and other code (MVP / MVVM / etc) would also help to structure the codebase.

misaochan commented 7 years ago

Thanks for the suggestions, all! I am on board with most of the technical changes suggested, especially refactoring the large and complex classes (ShareActivity especially would be top priority IMO), and switching the deprecated XML queries to JSON ones.

@psh and @maskaravivek , would you two like to collaborate to come up with a list of technical improvements that we would add to my user-focused list above? Preferably with input from @whym , @nicolas-raoul and anyone else who is interested, too.

@VojtechDostal , to what extent do you think WMF would be willing to fund technical improvements that would make the codebase significantly cleaner but have little or no impact on the end user? If, say, 33% of our proposed improvements were technical, would that be viewed as reasonable by WMF?

VojtechDostal commented 7 years ago

@misaochan I would not be afraid of including the "technical" changes, which do not affect the appearance or functionality, into the proposal. As long as they are well reasoned and are supplemented with changes that do matter for the user, I'm fine with everything. A good code is the basis for all future improvements.

misaochan commented 7 years ago

Wonderful, thanks for the clarification! :)

We can definitely go ahead with including some of the technical improvements, then. Personally, I would suggest that we focus first on the improvements that do not require adding new libraries, and then judiciously select which new libraries we want to include. This could be a personal preference, but my opinion is that the more libraries/external frameworks we include, the higher the barrier of entry to new contributors/volunteers. So IMO each new library is a trade-off that needs to be considered carefully.

That being said, I have not been doing Android development for very long, so if the more experienced devs come to a consensus that there is a very good reason for any or all of the libraries suggested, I'm happy to go with it.

psh commented 7 years ago

Classifying things according to "library" / "no library" gets us a list that looks something like this (based on details from @VojtechDostal with 2 additions)

No Library Needed Library Needed
Move to JSON based WIkimedia API (formerly, 'use Retrofit'). We already include OkHttp and GSON in the build. Use Glide library instead of asynctask load of images. Glide already developed for loading images, and its support different paths for imageview (URI, URL, file path at SD card and etc). Simple use, simple result
For fragments, use newInstance pattern instead of creating new objects. Fragment newInstance() pattern prevent memory leaks Add dagger implementations to project. Dagger is dependency injection library which will help to remove many initializations of some utils/loaders/api workers and etc. For example in current project it will help to initialize SharedPreferences client once for using in many parts of app. (the runtime for Dagger is tiny, it's mostly compile-time annotation processing)
Most common sizes must be moved to dimens.xml and separated by screen-sizes folders. Too much values are hardcoded in layouts Move layouts to ConstraintLayout where its possible, it much smoother and easy to operate. (but it's a Google support library)
Move icons to SVG format instead of PNG, which will decrease APK size and improve icon multi-screen support Design a first-run experience around the MaterialShowcaseView library. (#706)
User shrinkResources = true in build.gradle to remove unused resources from APK file on release.
Add rxAndroid to project - it will speedup application, remove logic from main thread and provide huge possibilities make multithreading apps (already underway)
Android data binding for activities, fragments, viewholders. Android data binding - powerful technique which simplify view code, and let you add some boilerplate logic to XML instead of some operations directly in activity, fragment, viewholder. Renderers can be replaced by Android data binding implementations
Address the very high cyclomatic complexity we are seeing from Codacy by breaking up Activities & Fragments according to one of the well-respected architectural patterns (MVP / MVVM / etc)

I'm being really strict with both Dagger and ConstraintLayout - both libraries are from Google and could be counted as "core" to the platform, but then again, they are additional items in build.gradle

misaochan commented 7 years ago

Thanks for the sorting, @psh ! I would also add your suggestion to the "no library needed" -

Codacy is reporting some very high cyclomatic complexity numbers for classes like ShareActivity and MediaDetailsFragment. I think we should take a look at breaking up some of these big / complex classes to make the code more maintainable. Some (architectural) separation between View, Model and other code (MVP / MVVM / etc) would also help to structure the codebase.

MaterialShowcaseView appears to have very tangible benefits for the user (which I am not sure how we could easily replicate without it), so I would vote that that particular library is worth adding. Dagger, Glide and ConstraintLayout is up to the rest of you.

sivaraam commented 7 years ago

We could see if the Wikipedia app allows us to do that,

Could you describe a little more about what you expect the app to allow you to do? This might help in assessing the feasibility.

and we should log the edits that are made via our app to ensure that we aren't vandalizing articles.

You could just tag the edits triggered from the app rather than logging it.

or we could also implement the functionality in our own app if other options don't suit.

I don't like the sound of that. It seems to be a feature creep that could waste developer time which could be used to in improving something else. It's better to leave that to the android app or mobile web.

This reminds me of ES file explorer which despite of claiming to be a file explorer implements all sorts of totally unrelated things. I find that to be an ugly decision.

Improve map navigation - implement compass and routing so that there is no need for user to switch to their map app for navigation. The idea is that the user can then do everything they need without leaving our app. Not sure how technically feasible this is - it might not be worth the work

Shortly (in the line of my previous comment), It's a feature creep and not worth the work. Just leave that to a navigation app. In case you try to implement navigation you'll have to address all sorts of issues regarding navigation which isn't something the app should be focusing on.

N.B. : These are just my opinions so please don't take them seriously.

maskaravivek commented 7 years ago

Hello, all! Already a lot of good suggestions for what we could include in the WMCZ proposal. @misaochan and @psh's list are very extensive and we probably just need to prioritize things in it for the proposal.

Few comments from the current discussion:

neslihanturan commented 7 years ago

Hello team, Firstly, I loved reading this thread. It is very well organised discussion of our distributed ideas. Thanks to all contributors and all the tables and bullets, everything is clarified.

I can add some small notes:

nicolas-raoul commented 7 years ago

@sivaraam While directions is probably overkill, compass on the Nearby map would be really helpful. As a heavy user of Nearby, I always head towards the nearest cluster of items, and only open a real map app when I don't know what is the direction I should head to. This is not feature creep, this is an essential feature if we are serious missing pictures.

janpio commented 7 years ago

(I don't know how "exact" this has to be, but if you can try to avoid "push notification" as it implies [complicated and extensive] technical implementation that is not really needed by both issues and features they refer to - "notification" explains the features well enough)

sivaraam commented 7 years ago

Yeah, @nicolas-raoul compass would be fine. I didn't think of providing compass alone as a feature, so I didn't mention that explicitly. I'm not pretty sure of it's feasibility, though.

misaochan commented 7 years ago

Great feedback, thanks all! I have modified the opening post to reflect some of the comments and to include the table of technical improvements made by @psh (with the exclusion of MaterialShowcaseDesign, which I think is a user-focused improvement and not a technical one).

I agree with @maskaravivek that we will need to prioritize things. Depending on the budget, it is unlikely that every single improvement on that list will be achievable. I will mark some items on the list as high-priority, feel free to go through and suggest other items that aren't marked. (This doesn't mean that lower-priority items won't be included, just that if we have to ditch anything, high priority items have a lower chance of being ditched)

Good points re: map navigation. I am inclined to agree that we can include the compass but exclude the routing/directions.

@sivaraam let us discuss the details surrounding the implementation of Wikipedia edits at #872 , so we can keep this issue for more general discussions. I will try and come up with exactly what sort of modification we might want and post there later.

psh commented 7 years ago

I added a few new issues and did a little organizing based on what @whym started - the project WMCZ Plans for 2018 has a "backlog" column of the various issues we called out in this thread. I propose that the ones we want to work on get pulled over into the "Ready To Start" and ordered from top-to-bottom in a priority ordering. As things are assigned we can drop them into "In Progress" and then "In Test" as they turn into PR's.

misaochan commented 7 years ago

Awesome, thanks guys! The system looks good to me. :)

(We should probably make a similar system for the IEG renewal if/when it is approved, too)

misaochan commented 7 years ago

Oh, I just noticed that a few of the items in that project were actually slated for the IEG renewal proposal (see #694 and its associated Metawiki link). Apologies for not being clear about the two - they are actually two separate proposals and two separate "grants". The 6-month IEG renewal was applied for in July 2017 and was intended to run in 2017. The WMCZ 2018 proposal was intended to fund development in 2018 after the IEG renewal completes and the deadline for this proposal is 1 Oct 2017.

It is a little bit confusing now, as the 2nd proposal is due soon but the 1st hasn't received an answer yet.

I have transferred the mistaken tasks to a new "IEG renewal" project, no worries. Just felt that I needed to clarify.

misaochan commented 6 years ago

This issue is outdated, please see #1683