Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

For #9299: Get download notification accent color from fenix to match theme #9300

Merged
merged 1 commit into from
Jan 6, 2021
Merged

For #9299: Get download notification accent color from fenix to match theme #9300

merged 1 commit into from
Jan 6, 2021

Conversation

hkaancaliskan
Copy link

@hkaancaliskan hkaancaliskan commented Dec 26, 2020

Fenix side: mozilla-mobile/fenix#17231

Warning to reviewer: I'm amateur. Please double check everything 😊

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

@hkaancaliskan hkaancaliskan changed the title For #9299: Get notification accent color from fenix to match theme For #9299: Get download notification accent color from fenix to match theme Jan 4, 2021
Copy link
Contributor

@Amejia481 Amejia481 left a comment

Choose a reason for hiding this comment

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

Thanks for this patch, great job, I added some comments for minor modification, additionally we could add an entry to the changelog file.

@Amejia481 Amejia481 added the ✏️ needs changes PRs that need some changes/fixes before they can land label Jan 5, 2021
@hkaancaliskan
Copy link
Author

@Amejia481 I'm ready for round 2 😊

@Amejia481 Amejia481 removed the request for review from csadilek January 6, 2021 14:21
docs/changelog.md Outdated Show resolved Hide resolved
@Amejia481
Copy link
Contributor

@hakkikaancaliskan great job, I think we only need to do some minor changes and squash the commits and it will be ready for lading 🚢 !

@hkaancaliskan
Copy link
Author

@Amejia481 I think it's all done, could you please re-check?

Copy link
Contributor

@Amejia481 Amejia481 left a comment

Choose a reason for hiding this comment

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

Outstanding work 🚢 !

@Amejia481 Amejia481 added 🛬 needs landing PRs that are ready to land and removed ✏️ needs changes PRs that need some changes/fixes before they can land labels Jan 6, 2021
@mergify mergify bot merged commit b779d7f into mozilla-mobile:master Jan 6, 2021
@hkaancaliskan hkaancaliskan deleted the notification-accent branch January 6, 2021 17:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🛬 needs landing PRs that are ready to land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants