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

Issue 44 resolved #48

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

vatsalshah041
Copy link

@vatsalshah041 vatsalshah041 commented Dec 27, 2023

Description

Please provide a meaningful description of what this change will do, or is for. Bonus points for including links to
related issues, other PRs, or technical references.

Note that by not including a description, you are asking reviewers to do extra work to understand the context of this
change, which may lead to your PR taking much longer to review, or result in it not being reviewed at all.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

vatsalshah041 and others added 2 commits November 11, 2023 22:29
…for laptops as well as mobiles

co-authored-by: Dhruv <[email protected]>
co-authored-by: Tanish <[email protected]>
co-authored-by: Vidhita <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a part of #42 and someone else is working on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this script? It is the same as run_local.sh right?

Copy link
Author

Choose a reason for hiding this comment

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

Actually, there are some changes in this so kept I this file

Copy link
Contributor

Choose a reason for hiding this comment

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

Changing the ports here breaks the application when we run it locally. The login/signup routes (USERS_URL) gives "not found" error and ATM_URL gives "cannot fetch" error. So please don't change the ports here.

@jashmehta3300
Copy link
Contributor

Hey! @vatsalshah041

Thank you for the PR. I have reviewed the commits and left a few comments.

In general, I think there are 2 main issues:

  • debugging code is present in many places, which should be cleaned.
  • change in ports is breaking the application when it is run locally.

Can you please have a look at those?

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