Skip to content

Conversation

@TimoPtr
Copy link
Member

@TimoPtr TimoPtr commented Dec 1, 2025

Summary

This PR removes TODOs that we don't need anymore and address some of them when it makes sense.

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Any other notes

No TODO should be left from the onboarding code base the last one is going to be address in #6115

The split of the tests are in a dedicated commit that can be ignored.

},
onNext = onOnboardingDone,
)
// TODO: Consider making auth_code a value class to prevent string parameter mismatches
Copy link
Member Author

Choose a reason for hiding this comment

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

I tried it (I have a stash with the changes). I've used a value class in the AuthRepository to enforce a strong type but then this types is not properly use by the navigation library and it needs a custom NavType mapping which I don't like.

I hope this is going to work in nav3 let see, for now I'm going to keep it like this.

if (isError) {
Text(
text = stringResource(commonR.string.manual_server_wrong_url),
// TODO probably wrong style and color/token
Copy link
Member Author

Choose a reason for hiding this comment

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

All TODOs regarding the color and sytle have been removed since the design team won't have time for us and for now it looks ok. We are going to need a more general change over the app later at least for the typographie.

@TimoPtr TimoPtr force-pushed the feature/onboarding-address-todos branch from 1a70c20 to c286eb1 Compare December 1, 2025 16:47
@TimoPtr TimoPtr changed the base branch from feature/use_new_onboarding to feature/onboarding-rework-onboardapp December 1, 2025 16:47
@TimoPtr
Copy link
Member Author

TimoPtr commented Dec 1, 2025

Last TODOs to address are to verify the URL for the documentation and it is going to be addressed in another PR.

@jpelgrom
Copy link
Member

jpelgrom commented Dec 2, 2025

Taking "No TODO should be left from the onboarding code base" very seriously:

Missed:

What about:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants