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

DetailScreen Complete #8

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

dungd200803btvn
Copy link

Related Tickets

WHAT

  • See detail weather forecast for next 24 hours and the 7 coming days

Evidence (Screenshot or Video)

image

Review Checklist

Category View Point Description Expected Reviewer Answer Self review Reviewer2 (name)
Conventions Does the code follow Sun* coding style and coding conventions? https://github.com/framgia/coding-standards/blob/master/eng/android/coding_convention_android_kotlin.md YES
  • yes
  • yes
  • Redmine Does the ticket follow Sun* Redmine working process? https://github.com/framgia/Training-Guideline/blob/master/WorkingProcess/redmine/redmine.md YES
  • yes
  • yes
  • Documentation Is there any incomplete code? If so, should it be removed or flagged with a suitable marker like ‘TODO’? YES
  • yes
  • yes
  • Notes (Optional)

    (Impacted Areas in Application(List features, api, models or services that this PR will affect))

    (Other notes)

    @dungd200803btvn dungd200803btvn changed the base branch from master to develop September 8, 2024 15:11
    Comment on lines 17 to 18
    baseApiService = get(named("baseApiService")),
    proApiService = get(named("proApiService")),
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Suggested change
    baseApiService = get(named("baseApiService")),
    proApiService = get(named("proApiService")),
    baseApiService = get(named("baseApiService")),
    proApiService = get(named("proApiService")),

    "baseApiService" & "proApiService" dùng ở nhiều nơi mình nên tạo biến const cho nó rồi dùng thay vì hard code như này e nhé!

    Copy link
    Contributor

    @daolq-2712 daolq-2712 left a comment

    Choose a reason for hiding this comment

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

    OK!

    @daolq-2712 daolq-2712 merged commit 77952bd into awesome-academy:develop Sep 9, 2024
    1 check passed
    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.

    2 participants