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

Commit for issue #12. #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

adhyay2000
Copy link

@adhyay2000 adhyay2000 commented Oct 24, 2020

modified the notify_mesej function to have chatroom argument, that gives the information about user to a specific chatroom and reduces the network overload.

…gives the information about the user in a specific chatroom, and reduces the network usage for clients.
@gridhead
Copy link
Owner

@adhyay2000 you are requested to allow some time to review your PR. Continually closing and opening of pull requests on your accord does not help the contributors and literally ends up wasting time which they spent on reviewing a PR which could not be merged as it was prematurely closed by the requester.

Thank you for understanding.

For your reference, @vinmay @shivangswain @VaibhavSaini19 @s-katte

@adhyay2000
Copy link
Author

adhyay2000 commented Oct 25, 2020

Hi,
I am sorry for closing the PR while it's still under review. After going through comments, I realized that server-side validation can only include server generating the chatroom name and client generating password themselves. However, this is now reduced to problem of not having same named chatroom, which is being addressed by @vivek-blip ,i guess in issue #37 . So, I closed it to avoid anyone's wastage of time. This commit is basically undoes the work done for server-side validation and only contains work for reducing the network usage, while querying the available message

@gridhead gridhead self-requested a review October 29, 2020 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants