-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature map #17
Feature map #17
Conversation
✔️ Deploy Preview for choose-wisely ready! 🔨 Explore the source changes: 9729bbf 🔍 Inspect the deploy log: https://app.netlify.com/sites/choose-wisely/deploys/611eaafee00b9200087d3672 😎 Browse the preview: https://deploy-preview-17--choose-wisely.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for next time keep the 2 PRs separated I can see changes from the other PR done here too
LGTM but I can't approve it now because it has changes from the other branch which will cause a conflict later, to solve it we either have to wait until the other one is merged or you can create another branch from dev, include only the map changes in it and open a new pr |
Thanks for the review, I'll wait until the other PR is merged and be careful next time to work on one branch per PR. |
@Dkay-code there are some conflicts need to be solved here before merging |
PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Close issue #8 The map section is finished including a map with the number of universities by province.
A test folder was added to src and a snapshot test has been created under the name 'Map.test.js' for this section.