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

ci: Return to entrypoint based scanner #3874

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Conversation

M123-dev
Copy link
Member

What

  • Nothing special, removing the switch in the dev settings. Now the scanner is again dependent on the entry point which is used.
  • I removed the red text above the scanner
  • Now we don't use a provider for the scanner type anymore. It won't change during runtime, so I decided to go with a global var

This will likely be a bit restructured in my next PR to allow building completely without ml kit for F-Droid

@M123-dev M123-dev requested a review from a team as a code owner April 14, 2023 13:43
@github-actions github-actions bot added 📈 Analytics We use Sentry and Matomo, with an opt-in system dependencies 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… 🧪 Tests labels Apr 14, 2023
Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Hi @M123-dev!

I'm really not a big fan of import 'main.dart' whose local variables are being used as global variables.
At least a dedicated class for global variables, so that human beings can read code and understand where scannerType or flavour come from?
Not sure either to which extent global variables are recommended in dart/flutter, but that's another story.

Let's go to the next step!

@M123-dev M123-dev merged commit 1734a20 into develop Apr 14, 2023
7 checks passed
@M123-dev M123-dev deleted the entrypoint-based-scanner branch April 14, 2023 15:07
@M123-dev
Copy link
Member Author

Thanks a lot @monsieurtanuki, yes I agree we should move it to a dedicated file, though probably at least the scanner type one will disappear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📈 Analytics We use Sentry and Matomo, with an opt-in system dependencies 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… 🧪 Tests
Development

Successfully merging this pull request may close these issues.

None yet

2 participants