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!: [MOBC-701] [MOBC-702] combine deriv_auth_ui with deriv_auth and remove deriv_auth_ui #388

Merged
merged 7 commits into from
Jan 8, 2024

Conversation

ahrar-deriv
Copy link
Contributor

@ahrar-deriv ahrar-deriv commented Dec 27, 2023

Clickup link:
https://app.clickup.com/t/20696747/MOBC-701
https://app.clickup.com/t/20696747/MOBC-702

This PR contains the following changes:

  • combines deriv_auth_ui with deriv_auth
  • removes deriv_auth_ui from packages
  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Pre-launch Checklist (For PR creator)

As a creator of this PR:

  • ✍️ I have included clickup id and package/app_name in the PR title.
  • 👁️ I have gone through the code and removed any temporary changes (commented lines, prints, debug statements etc.).
  • ⚒️ I have fixed any errors/warnings shown by the analyzer/linter.
  • 📝 I have added documentation, comments and logging wherever required.
  • 🧪 I have added necessary tests for these changes.
  • 🔎 I have ensured all existing tests are passing.

Reviewers

@bassam-deriv @behnam-deriv @ernest-deriv

Pre-launch Checklist (For Reviewers)

As a reviewer I ensure that:

  • ✴️ This PR follows the standard PR template.
  • 🪩 The information in this PR properly reflects the code changes.
  • 🧪 All the necessary tests for this PR's are passing.

Pre-launch Checklist (For QA)

  • 👌 It passes the acceptance criteria.

Pre-launch Checklist (For Maintainer)

  • [MAINTAINER_NAME] I make sure this PR fulfills its purpose.

@ahrar-deriv ahrar-deriv marked this pull request as ready for review December 28, 2023 08:24
Copy link
Contributor

@bassam-deriv bassam-deriv left a comment

Choose a reason for hiding this comment

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

@ahrar-deriv I suggest that we just merge this PR for now since it doesn't contain any actual changes to the code and all we did was move everything from deriv_auth_ui to deriv_auth which is technically not a breaking change so no need to keep it opened. Next we create a new PR which contains the actual refactoring of the package which will actually have creaking changes.
I do suggest though to rename the PR from refactor to feat since we're moving a whole feature (the UI) to this package just for the sake of properly versioning it once this PR is merged.

@behnam-deriv
Copy link
Contributor

@ahrar-deriv right now deriv_auth.dart barrel file only exports logic related files. we should add the UI related files also. This way client can just import deriv_auth.dart and have access to logic and UI for authentication and no need for additional import lines.

@ahrar-deriv ahrar-deriv changed the title refactor(deriv_auth): (WIP) combine deriv_auth_ui with deriv_auth feat(deriv_auth): (WIP) combine deriv_auth_ui with deriv_auth Jan 2, 2024
@ahrar-deriv ahrar-deriv changed the title feat(deriv_auth): (WIP) combine deriv_auth_ui with deriv_auth refactor!: combine deriv_auth_ui with deriv_auth and remove deriv_auth_ui Jan 3, 2024
@ahrar-deriv ahrar-deriv changed the title refactor!: combine deriv_auth_ui with deriv_auth and remove deriv_auth_ui refactor!: [MOBC-701] [MOBC-702] combine deriv_auth_ui with deriv_auth and remove deriv_auth_ui Jan 3, 2024
@bassam-deriv
Copy link
Contributor

@ahrar-deriv Why did you add ! to the PR title?
I don't think this is a breaking change for deriv_auth, and deriv_auth_ui is deleted now so there is no package that needs a breaking change version bump.

@ahrar-deriv
Copy link
Contributor Author

@ahrar-deriv Why did you add ! to the PR title? I don't think this is a breaking change for deriv_auth, and deriv_auth_ui is deleted now so there is no package that needs a breaking change version bump.

need to bump the major as this is adding a new package ( feat ) in and removing another package, so it is easier to spot in versions also (in tags)

…ges into combine_ui_with_auth

# Conflicts:
#	README.md
#	packages/deriv_auth_ui/CHANGELOG.md
#	packages/deriv_auth_ui/pubspec.yaml
@ahrar-deriv ahrar-deriv merged commit 853bcbf into deriv-com:master Jan 8, 2024
2 checks passed
@ahrar-deriv ahrar-deriv deleted the combine_ui_with_auth branch January 8, 2024 06:55
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