Skip to content
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: background tasks with classes #2994

Merged

Conversation

monsieurtanuki
Copy link
Contributor

New files:

  • abstract_background_task.dart: Abstract background task.
  • background_task_details.dart: Background task that changes product details (data, but no image upload).
  • background_task_image.dart: Background task about product image upload.

Impacted files:

  • add_basic_details_page.dart: refactored the call to background task; minor refactoring
  • background_task_helper.dart: moved most of the code to new classes AbstractBackgroundTask and offsprings.
  • edit_ingredients_page.dart: refactored the call to background task
  • nutrition_page_loaded.dart: refactored the call to background task
  • picture_capture_helper.dart: refactored the call to background task
  • simple_input_page.dart: refactored the call to background task
  • simple_input_page_helpers.dart: added and implemented method getTask

What

  • The goal is to refactor and make easier the use of background tasks. Currently implemented: image upload and product changes.

Part of

New files:
* `abstract_background_task.dart`: Abstract background task.
* `background_task_details.dart`: Background task that changes product details (data, but no image upload).
* `background_task_image.dart`: Background task about product image upload.

Impacted files:
* `add_basic_details_page.dart`: refactored the call to background task; minor refactoring
* `background_task_helper.dart`: moved most of the code to new classes `AbstractBackgroundTask` and offsprings.
* `edit_ingredients_page.dart`: refactored the call to background task
* `nutrition_page_loaded.dart`: refactored the call to background task
* `picture_capture_helper.dart`: refactored the call to background task
* `simple_input_page.dart`: refactored the call to background task
* `simple_input_page_helpers.dart`: added and implemented method `getTask`
@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2022

Codecov Report

Merging #2994 (d2d614c) into develop (d2f8077) will increase coverage by 0.01%.
The diff coverage is 0.60%.

@@            Coverage Diff             @@
##           develop   #2994      +/-   ##
==========================================
+ Coverage     6.50%   6.51%   +0.01%     
==========================================
  Files          241     244       +3     
  Lines        12031   12026       -5     
==========================================
+ Hits           783     784       +1     
+ Misses       11248   11242       -6     
Impacted Files Coverage Δ
...h_app/lib/background/abstract_background_task.dart 0.00% <0.00%> (ø)
...ooth_app/lib/background/background_task_image.dart 0.00% <0.00%> (ø)
...smooth_app/lib/helpers/background_task_helper.dart 0.00% <0.00%> (ø)
...smooth_app/lib/helpers/picture_capture_helper.dart 0.00% <0.00%> (ø)
..._app/lib/pages/product/add_basic_details_page.dart 0.00% <0.00%> (ø)
...h_app/lib/pages/product/edit_ingredients_page.dart 0.00% <0.00%> (ø)
...h_app/lib/pages/product/nutrition_page_loaded.dart 0.00% <0.00%> (ø)
...mooth_app/lib/pages/product/simple_input_page.dart 0.00% <0.00%> (ø)
...p/lib/pages/product/simple_input_page_helpers.dart 0.00% <0.00%> (ø)
...th_app/lib/background/background_task_details.dart 1.61% <1.61%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@AshAman999 AshAman999 left a comment

Choose a reason for hiding this comment

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

Honestly, I am impressed, thanks a lot for the great refactoring
I just left a real few minor issues, Just come better comments suggestions

Impacted files:
* `abstract_background_task.dart`
* `add_basic_details_page.dart`
Copy link
Member

@AshAman999 AshAman999 left a comment

Choose a reason for hiding this comment

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

Thank you @monsieurtanuki for this PR
Totally worth something to learn from

@monsieurtanuki monsieurtanuki merged commit 68b6939 into openfoodfacts:develop Sep 10, 2022
@monsieurtanuki
Copy link
Contributor Author

Thank you @AshAman999 for your enthusiastic review!
For the record I found your initial decision to "reject" my PR excessive, if I write in British understatement mode.
Please do reject my PRs when there's something dangerous for the app integrity or consistency, but not for a couple of typos in comments. Just "comment" or "approve", that won't prevent me from fixing whatever you pointed out, if relevant.
The good news is that you liked the rest of the code! ;)

@AshAman999
Copy link
Member

For the record I found your initial decision to "reject" my PR excessive, if I write in British understatement mode.
Please do reject my PRs when there's something dangerous for the app integrity or consistency, but not for a couple of typos in comments. Just "comment" or "approve", that won't prevent me from fixing whatever you pointed out, if relevant.

I didn't give much thought when selecting the review dropdown, honestly, to me all of them looked the same
I will be better responsible with the tags, sorry for the inconvenience though

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.

3 participants