search

android / nowinandroid

A fully functional Android app built entirely with Kotlin and Jetpack Compose
Apache License 2.0
16.61k stars 3.01k forks source link

Dt/clean arch #1487

Closed nabond251 closed 3 months ago

nabond251 commented 4 months ago

What I have done and why

Updated dt/clean-arch from latest main 85129e so that it can continue to be a useful, up-to-date reference in exploring Clean Architecture per #1277 and #1273.

It does not look like tests are currently all passing. Possible I'm not configured correctly. In my dev environment, I have above main passing 70/133 tests; prior dt/clean-arch passing 61/120 tests; I'm thinking that's roughly parity with my updates passing 63/106?

google-cla[bot] commented 4 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

JackEblan commented 4 months ago

This really needs a bigger refactoring just to compliment the Clean Architecture.

:core:analytics :core:common :core:data :core:data-test :core:database :core:datastore :core:datastore-test :core:datastore-proto :core:screenshot-testing :core:ui :core:network :core:notifications :core:testing

Are part of the Infrastructure Layer and we need to move these outside of the core.

Also, there are some Android Frameworks included in the Data layer util and we need to move these Android Frameworks into a separate modules, just being outside of the :core

I'm not sure if they're going to like this.

nabond251 commented 4 months ago

@JackEblan thanks for the feedback. Just for context (which should have probably been provided in the opening line, my bad/will edit), I'm looking to update not main proper but #1277, which is an exploration of clean architecture prompted by #1273. Neither that nor this are full and final statements on Clean Architecture proper, otherwise those aspects you point out probably should be dealt with as well.

dturner commented 3 months ago

This isn't something we're looking to support/merge for now. If you wish to investigate a clean architecture version of this project please do it on your own fork.