-
Notifications
You must be signed in to change notification settings - Fork 306
feat: prevent typing over-long topic names #1230
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
Conversation
Signed-off-by: ARYPROGRAMMER <[email protected]>
Thanks. Before we can review this PR, it will need (a) tests, and (b) to remove extraneous changes so that the commit is readable. See our README: |
surely greg, the file was auto formatted thus it happened so. For the tests, I'll just look into it and fix it |
Signed-off-by: ARYPROGRAMMER <[email protected]>
Signed-off-by: ARYPROGRAMMER <[email protected]>
Signed-off-by: ARYPROGRAMMER <[email protected]>
@gnprice lgtm now , go ahead with the review. Let me know further |
hey greg any updates? |
This still has extraneous changes which you'll need to remove before we can review it. See the link I posted above, and see in particular the Zulip guide to clear and coherent commits which is linked from there. There's also no need to re-ping after a day. Reviewers have weekends, and holidays, and also other work responsibilities. If it's been a week or two since you requested review and there's no reply, then it can be appropriate to bump the thread. |
Signed-off-by: Arya Pratap Singh <[email protected]>
Signed-off-by: Arya Pratap Singh <[email protected]>
let me know whether I should pull changes from #1239 and add them to this PR |
@chrisbobbe any updates on this |
#1239 has been merged! 🎉 |
what changes are needed now in this? just rebase? |
i'll jjust have to pull the new compose box and its test and add my current pr changes in it, is this correct or do we need something more? |
You should rebase your commits on top of current |
Signed-off-by: Arya Pratap Singh <[email protected]>
i have made the changes required in the file but maybe i would need to open a new pr for this to get rebased safely, kindly provide suggestions. thanks |
or if you can remove merge commits or squash everything then i think this will work. let me know further |
There's no need to open a new PR; just force-push here: https://zulip.readthedocs.io/en/latest/git/using.html#force-push-changes-to-github-after-you-ve-altered-your-history |
i mean i already created a new pr so can we go with that, i'll have to setup this up again in local if i have to do force push PS: since that one is closed i need to figure this out now |
Signed-off-by: Arya Pratap Singh <[email protected]>
having some difficulties ,check once please. git push origin +feat/compose I ran this. maybe still there are issues. let me know further |
|
Thanks @ARYPROGRAMMER for your work on this so far. We're now focusing more strictly on launch issues until the launch is ready, and this is a post-launch issue. It doesn't seem like this PR has reached the point yet of being possible for us to review, so I'll go ahead and close it. |
This PR adds the functionality mentioned in the Issue #1218
Demo Video on a real android device:
demo.mp4