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

Rebase and simplification of #870 #1161

Closed
wants to merge 7 commits into from

Conversation

neiljp
Copy link
Collaborator

@neiljp neiljp commented Mar 7, 2022

What does this PR do?

This is a rebase and simplification of #870, which is a partial fix for #774.

I'd appreciate feedback from anyone who contributed to that issue.

Tested?

  • Manually
  • Existing tests (adapted, if necessary)

Commit flow

After rebasing and fixing formatting, this splits the original PR into a refactor and feature commit. Around these there are various simplifying commits, most of which I expect to squash with those two before merging - though I may leave them in this PR for future reference by others in terms of tidying a previous PR.

Notes & Questions

This doesn't fully fix the issue, since it doesn't handle private messages, so anyone is welcome to take that up separately.

This also exposes how we could further simplify and refactor the compose code, though considering older PRs may be good to do to avoid them bit-rotting further too :)

@neiljp neiljp added this to the Next Release milestone Mar 7, 2022
@zulipbot zulipbot added size: L [Automatic label added by zulipbot] enhancement New feature or request labels Mar 7, 2022
@neiljp neiljp added the PR ready to be merged PR has been reviewed & is ready to be merged label Mar 9, 2022
@neiljp
Copy link
Collaborator Author

neiljp commented Mar 13, 2022

This is now merged as 80bb9c0 and the two prior commits, so I'm going to manually close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: message editing enhancement New feature or request PR ready to be merged PR has been reviewed & is ready to be merged size: L [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants