-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/octopus #8
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
Conversation
Reviewer's Guide by SourceryThis pull request migrates the project to use the Sequence diagram for the new user login processsequenceDiagram
participant User
participant AuthBloc
participant AuthRepository
participant AuthRemoteDataSource
participant SecureStorageManager
User->>AuthBloc: LoginAuthEvent(username, password)
AuthBloc->>AuthRepository: login(username, password)
AuthRepository->>AuthRemoteDataSource: login(username, password)
AuthRemoteDataSource-->>AuthRepository: TokenPair
AuthRepository->>SecureStorageManager: setToken(TokenPair)
SecureStorageManager-->>AuthRepository: void
AuthRepository-->>AuthBloc: TokenPair
AuthBloc-->>User: Authentication successful
Sequence diagram for the logout processsequenceDiagram
participant User
participant AuthBloc
participant AuthRepository
participant SecureStorageManager
User->>AuthBloc: LogoutAuthEvent()
AuthBloc->>AuthRepository: logout()
AuthRepository->>SecureStorageManager: deleteToken()
SecureStorageManager-->>AuthRepository: void
AuthRepository-->>AuthBloc: void
AuthBloc-->>User: Logout successful
Updated class diagram for DependenciesContainerclassDiagram
class DependenciesContainer {
-SharedPreferences sharedPreferences
-PackageInfo packageInfo
-RestClientBase restClient
-AuthBloc authBloc
-UserCubit userCubit
-SettingsBloc settingsBloc
+DependenciesContainer(
SharedPreferences sharedPreferences,
PackageInfo packageInfo,
RestClientBase restClient,
AuthBloc authBloc,
UserCubit userCubit,
SettingsBloc settingsBloc
)
+String toString()
}
Updated class diagram for SettingsBlocclassDiagram
class SettingsBloc {
-ILocaleRepository _localeRepo
-IThemeRepository _themeRepo
+SettingsBloc(ILocaleRepository localeRepository, IThemeRepository themeRepository, SettingsState initialState)
+Future<void> _updateTheme(UpdateThemeSettingsEvent event, Emitter<SettingsState> emit)
+Future<void> _updateLocale(UpdateLocaleSettingsEvent event, Emitter<SettingsState> emit)
}
class SettingsEvent {
<<abstract>>
}
class UpdateThemeSettingsEvent {
-AppTheme appTheme
+UpdateThemeSettingsEvent(AppTheme appTheme)
}
class UpdateLocaleSettingsEvent {
-Locale locale
+UpdateLocaleSettingsEvent(Locale locale)
}
SettingsBloc --|> Bloc
SettingsEvent --|> Equatable
UpdateThemeSettingsEvent --|> SettingsEvent
UpdateLocaleSettingsEvent --|> SettingsEvent
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @yelmuratoff - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a state management solution like Riverpod or BLoC for managing the app's state, especially for settings and user authentication.
- The project has undergone a significant refactor, consider adding a summary of the changes in the description.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// List of extensions for `BuildContext` | ||
| extension ContextExtension on BuildContext { | ||
| /// `pop` pops the current `BuildContext` from the `Navigator`. | ||
| void pop() { |
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.
question: Clarify the order and rationale for multiple pop calls.
The pop() extension calls octopus.pop() before attempting Navigator.maybePop. It might be helpful to ensure that this order is intentional and that both are needed, so that unexpected behavior in navigation can be avoided.
| [developing packages and plugins](https://flutter.dev/to/develop-packages). | ||
| --> | ||
|
|
||
| TODO: Put a short description of the package here that helps potential users |
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.
suggestion: Remember to fill in the TODO placeholders in the README.
The README contains several TODOs. Please replace them with actual content.
Suggested implementation:
This package provides a collection of customizable user interface components and utilities designed for building responsive Flutter applications. It includes a variety of widgets, themes, and animations that streamline the development of modern, visually appealing UIs.
For every other TODO found in the README, ensure that you replace these placeholders with concrete explanations, documentation links, or usage examples that provide value to the potential users of the package.
| @@ -0,0 +1,3 @@ | |||
| ## 0.0.1 | |||
|
|
|||
| * TODO: Describe initial release. | |||
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.
suggestion: Update the changelog with a description of the initial release.
No description provided.