Skip to content

Conversation

Anshu370
Copy link

Notifications for user join/leave and Message Received events in chat application

Copy link
Owner

@Vikranth3140 Vikranth3140 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Anshu370,

The updates look good to me! I have a few follow-up points to ensure everything is in line:

  1. Could you please provide a quick demo video showing the updated application in action? It would help us validate the recent changes visually.

  2. Regarding requirements.txt, could you confirm if it was generated specifically for this application? Were you working within a virtual environment before running pip freeze, or does it include other dependencies outside of this project?

  3. Also, I noticed the port was changed from 12345 to 1234. Was there a specific reason for this adjustment?

Thanks for your work on this!

@Anshu370
Copy link
Author

Anshu370 commented Nov 5, 2024

Hey @Vikranth3140 ,

  1. Here Is Clip of notification.
2024-11-05.09-23-08.mp4
  1. yes I have generated requirements.txt specifically for the Chat Application in virtual environment.

  2. There is no specific reason to change the port from 12345 to 1234 i have testing application in different computer so i have changed it.

Copy link
Author

@Anshu370 Anshu370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have change it to 1234 for testing in multiple computers.

@Anshu370 Anshu370 requested a review from Vikranth3140 November 5, 2024 04:12
Copy link
Owner

@Vikranth3140 Vikranth3140 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, lgtm, i have some changes which i'll do.

A few more points:

  1. What timestamp is the notification coming.
  2. Update the README and add notifications.
  3. Add a new issue explaining the problem you are trying to solve.
  4. Add the tkinter in requirements.txt

@Anshu370
Copy link
Author

Anshu370 commented Nov 8, 2024

okay in will do the changes.

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.

2 participants