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

Closes #12190: Create a middleware that allows to change tabs priority. #12192

Merged
merged 1 commit into from
May 26, 2022
Merged

Closes #12190: Create a middleware that allows to change tabs priority. #12192

merged 1 commit into from
May 26, 2022

Conversation

Amejia481
Copy link
Contributor

@Amejia481 Amejia481 commented May 18, 2022 •
edited
Loading

Put this as a draft to make sure I captured all the requirements on #12190, I'll update with tests and a changelog entry after.

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.

@Amejia481 Amejia481 added the 🕵️‍♀️ needs review PRs that need to be reviewed label May 18, 2022
@Amejia481 Amejia481 requested a review from jonalmeida May 18, 2022 21:14
@Amejia481
Copy link
Contributor Author

I'll update the draft to reflect this clarification mozilla-mobile/fenix#25212 (comment)

@Amejia481 Amejia481 force-pushed the issue-12190 branch 2 times, most recently from 3934b63 to 6c83b7e Compare May 20, 2022 19:29
@Amejia481
Copy link
Contributor Author

Amejia481 commented May 20, 2022 •
edited
Loading

@jonalmeida 👋🏽
I updated the PR for when you have some time :)
Thanks in advance!

@Amejia481 Amejia481 requested a review from jonalmeida May 20, 2022 19:32
jonalmeida
jonalmeida approved these changes May 20, 2022
View reviewed changes
Copy link
Contributor

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

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

Spoke to Arturo offline: This looks good to me! Just needs tests and would be nice to have this in the Sample Browser too.

✅

@Amejia481 Amejia481 marked this pull request as ready for review May 25, 2022 20:39
@Amejia481 Amejia481 requested a review from csadilek as a code owner May 25, 2022 20:39
@mergify
Copy link
Contributor

mergify bot commented May 26, 2022

This pull request has conflicts when rebasing. Could you fix it @Amejia481? 🙏

@Amejia481 Amejia481 added 🛬 needs landing PRs that are ready to land and removed 🕵️‍♀️ needs review PRs that need to be reviewed labels May 26, 2022
jonalmeida
jonalmeida approved these changes May 26, 2022
View reviewed changes
Copy link
Contributor

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

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

Still looks good to me! Some small nits with the doc, but please land it. 🎉

components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt Outdated Show resolved Hide resolved
components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt Outdated Show resolved Hide resolved
components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt Outdated Show resolved Hide resolved
@Amejia481
Copy link
Contributor Author

As we have to land before cutting beta 102, I'll this please feel free to add any comments!

@Amejia481 Amejia481 removed the 🛬 needs landing PRs that are ready to land label May 26, 2022
@Amejia481
Copy link
Contributor Author

Sorry, I just saw your comments 😅 . I'll address them

@Amejia481 Amejia481 added the 🛬 needs landing PRs that are ready to land label May 26, 2022
@Amejia481 Amejia481 removed the request for review from csadilek May 26, 2022 13:23
@Amejia481
Copy link
Contributor Author

Amejia481 commented May 26, 2022 •
edited
Loading

Thanks for the feedback, I addressed the comments and labeled it for landing :)

@mergify mergify bot merged commit 528016d into mozilla-mobile:main May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonalmeida jonalmeida jonalmeida approved these changes

Assignees
No one assigned
Labels
🛬 needs landing PRs that are ready to land
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Amejia481 @jonalmeida