-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
refactor: Make smooth_app a module (also called step 1) #3101
refactor: Make smooth_app a module (also called step 1) #3101
Conversation
Android impl is OK. Still have to check for iOS and tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @g123k!
I approve your code regarding the "initial step 1", i.e. adding a package
parameter to the asset file references in dart files. The rest is beyond my added value.
@@ -5,7 +5,7 @@ import android.os.Build | |||
import android.service.quicksettings.Tile | |||
import android.service.quicksettings.TileService | |||
import androidx.annotation.RequiresApi | |||
import org.openfoodfacts.scanner.MainActivity | |||
import org.openfoodfacts.app.MainActivity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't worry, this is OK for this one.
This is the name of the app
VS smooth_app
Android app
It seems that everything is now OK. No new feature is available in this step |
Is there someone to review/merge this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't double checked everything (this is a mobile review)
Just the .gitingore and I noticed we have a Linux macOS and Windows folder in /app/
We probably don't need that
@@ -0,0 +1,46 @@ | |||
# Miscellaneous |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the global .gitignore enouph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review Marvin.
For the "desktop" folders, there is an issue about the support of those platforms.
We won't release on them, but it will allow us to run the app without an emulator/simulator.
For the .gitignore, we have the same file in the smooth_app
package. It's actually the generated one.
Codecov Report
@@ Coverage Diff @@
## develop #3101 +/- ##
==========================================
+ Coverage 6.40% 6.44% +0.04%
==========================================
Files 247 248 +1
Lines 12324 12337 +13
==========================================
+ Hits 789 795 +6
- Misses 11535 11542 +7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This PR implemented the "Step 1" of the migration:
package
attribute for all assets filesapp
package (empty for now)smooth_app
as a dependencyNew entry points are: