Closed SnowVolf closed 10 months ago
@TrianguloY Fixed. PS: About LWP activity deletion:
This change requires a change of base implementation of the getMode() method inside ScreenManager. It potentially affects many states of this screen. I think this should be left for a future refactor
@SnowVolf
This change requires a change of base implementation of the getMode() method inside ScreenManager. It potentially affects many states of this screen. I think this should be left for a future refactor
Understood. You can keep it for now. In these cases I like to add a TODO comment (in this case in the javadoc, for the whole class) to not forget in the future. I can add an issue if you prefer it (some people don't like TODOs)
I added one last comment to fix, since in one of the cleanups you removed a necessary branch.
Also, while working on a public pr (specially if it has been reviewed) please avoid rebasing or changing the branch history, otherwise it's much harder for me to properly review the changes. Thanks! If you want the pr to only consist on a single commit after merging just say so and I'll squash&merge it. Or you can mark individual commits for fixup (I think git has a standard way, using the same title and prefixing with "fixup! ").
Last: there seems to be some conflicts, although they appear to be easily fixable. I can fix them myself if you prefer (if so, tell me)
@TrianguloY
Understood. You can keep it for now. In these cases I like to add a TODO comment (in this case in the javadoc, for the whole class) to not forget in the future. I can add an issue if you prefer it (some people don't like TODOs)
ok. I'm added todo
Also, while working on a public pr (specially if it has been reviewed) please avoid rebasing or changing the branch history, otherwise it's much harder for me to properly review the changes. Thanks!
I follow the rule: 1 PR - 1 commit. I haven't seen a merge squash function on Github (like GitLab). If you are not too lazy to combine them every time, okay.
here seems to be some conflicts
Fixed
@SnowVolf
I follow the rule: 1 PR - 1 commit. I haven't seen a merge squash function on Github (like GitLab). If you are not too lazy to combine them every time, okay.
For me is just choosing one option instead of the other :) so really don't worry (and as I said it helps enormously when reviewing, otherwise you can't compare and need to review the whole modified file again).
Everything seems fine now. Merging!
This PR closes #13 Also removed obsolete wear modules