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

Navigate back press fix #433

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jojonarte
Copy link
Contributor

@jojonarte jojonarte commented Apr 23, 2018

Fix for Issue #334

@codeguru42
Copy link
Member

@jojonarte What is the state of this branch? Do you need to write any tests or are the existing tests sufficient?

@jojonarte
Copy link
Contributor Author

@codeguru42 I want to know your opinion with my tests on this pr. As of the moment I believe I've test as much as I know for backpress.

@codeguru42
Copy link
Member

@jojonarte Please edit the message for your first commit to something more descriptive. It wasn't immediately obvious to me that it has the tests for this issue. I will take a look at them in more detail later this week.

@jojonarte jojonarte force-pushed the navigate_back_press_fix branch 2 times, most recently from 969c94d to c3e60dc Compare May 16, 2018 14:36
@codeguru42
Copy link
Member

When you have time to continue, this should be the next thing. Merge with devel/android and resolve conflicts. Then let me know when it is ready for review.

@jojonarte
Copy link
Contributor Author

@codeguru42 okay thanks

@codeguru42 codeguru42 added this to the 0.6.8 milestone Aug 27, 2018
@codeguru42
Copy link
Member

@jojonarte Is there anything else you need to do on this? Or is it ready for a final review?

@jojonarte
Copy link
Contributor Author

@codeguru42 there's still some things I have to fix.

@codeguru42
Copy link
Member

@jojonarte Okay. Just ping me when you are finished.

@jojonarte
Copy link
Contributor Author

@codeguru42 it's ready for review now.

@codeguru42
Copy link
Member

Does 9c82268 address the problem from #429?

@jojonarte
Copy link
Contributor Author

@codeguru42 it doesn't yet, I'll fix that also don't review yet. Will let you know.

@codeguru42
Copy link
Member

@jojonarte Just to be clear, do you mean you will fix that on this branch? If so, let's close #429.

@jojonarte
Copy link
Contributor Author

@codeguru42 yes

@codeguru42
Copy link
Member

codeguru42 commented Aug 28, 2018

@jojonarte Hold that thought. Now that I've looked at #429 more closely, let's keep that separate. I'll review what you have here and merge it so that you can reuse any code you need for fixing #429. Go ahead and work on your other issue for now.

@jojonarte
Copy link
Contributor Author

@codeguru42 okay

@codeguru42 codeguru42 modified the milestones: 0.6.8, 0.6.9 Sep 9, 2018
@jojonarte
Copy link
Contributor Author

Is there anything left for this PR?

@codeguru42
Copy link
Member

codeguru42 commented Nov 23, 2018

@jojonarte Thanks for following up on this. This PR is good for the moment. I am not ready to merge it yet until I finish some work I'm doing with #348. It will probably be a while before that one is finished and this one can be merged. In the mean time, feel free to work on #308 or another issue.

@codeguru42 codeguru42 changed the base branch from devel/android to main September 26, 2021 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants