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
Added 'is_landlocked' property #56
Conversation
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.
Looks promising! I added few comments/suggestions to improve your code 😉
Thank you @jav7zaid!
@jav7zaid you don't need to answer to every single comment - it spams my e-mail so hard 😅... If you decide that you will apply changes, you can just do them and after you finish/commit mark a comment as resolved 😉 |
@bhatvikrant and @sthiepaan please check the PR ,i have updated it with review comments. |
@jav7zaid after you finish applying and pushing changes - instead of writing a comment - you can click on little circle icon in Reviewers section (located at top right of the Pull Request) near reviewer nickname 😉 |
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
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.
@jav7zaid please resolve conflicts and then its good to go!
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.
I would like to merge this Pull Request but unfortunately we struggle with some merge conflicts. Conflicts are in:
README.md
data/data.json
index.js
spec/getCountriesSpec.js
@jav7zaid could you resolve them? 😉
done @sthiepaan |
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
i think my changes disturbed the existing test cases fixing them. |
@jav7zaid actually yes, you broke it! 😅... Always run |
Issue #47
#Pull Request description