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

feat: Add location addresses data from Korea #445 #991

Closed
wants to merge 6 commits into from

Conversation

GitKimUser
Copy link

  • Update location.cpp
  • Update location_data.h
  • Update location_test.cpp

@cieslarmichal cieslarmichal linked an issue Nov 23, 2024 that may be closed by this pull request
Copy link
Owner

@cieslarmichal cieslarmichal left a comment

Choose a reason for hiding this comment

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

why did you add some submodule? please remove

@GitKimUser
Copy link
Author

why did you add some submodule? please remove

image
do I have to remove all of the submodule?

@cieslarmichal
Copy link
Owner

Looks like you added faker-cxx as submodule, look in PR changes

@GitKimUser
Copy link
Author

Looks like you added faker-cxx as submodule, look in PR changes

image
I think that ./faker-cxx/faker-cxx is the directory that makes problem.. right??

@cieslarmichal
Copy link
Owner

Yes try to delete it from changes

@cieslarmichal
Copy link
Owner

also resolve conflicts

@GitKimUser
Copy link
Author

also resolve conflicts

I copied the codes and put them in as they were to resolve the conflict, but I checked and found out that this was not the right method. So, when I ran git merge origin/merge to get the latest source, the message 'are ready up to date' was already displayed, so I couldn't get the updated codes while I was working on it. Do you know how to get the latest source, update it, and commit again to avoid conflict in this situation?

@cieslarmichal
Copy link
Owner

I would try this: https://stackoverflow.com/questions/65215051/how-to-pull-the-changes-from-main-branch-to-forked-branch - it should work, if not join our discord channel and we will help you

@GitKimUser
Copy link
Author

GitKimUser commented Nov 25, 2024

I would try this: https://stackoverflow.com/questions/65215051/how-to-pull-the-changes-from-main-branch-to-forked-branch - it should work, if not join our discord channel and we will help you

I saw the contents of the stackoverflow you provided, tried it, and changed it again. I made several commitments, but I forgot to push, so I pushed all at once. I wonder if the conflict has been resolved about the change. If it hasn't been resolved, do I have to use git push origin feature/kim as the name of the branch??

@cieslarmichal
Copy link
Owner

test for koea fails: LocationTest.shouldGenerateKoreaStreet
also run clang format

@GitKimUser
Copy link
Author

GitKimUser commented Nov 26, 2024

test for koea fails: LocationTest.shouldGenerateKoreaStreet also run clang format

image

I've tried building and ctest without any code modification at the moment, but it passed with the screenshot. Regarding this test, is it irrelevant to the error above? Or was there a version update or correction for the ctest?

@cieslarmichal
Copy link
Owner

I have merged main branch into yours here, you can try fetching it and trying to run tests

@GitKimUser
Copy link
Author

I have merged main branch into yours here, you can try fetching it and trying to run tests

image

You mean the process of this screenshot, right? If so, when I ran the ctest again, it said that it didn't fail.

@GitKimUser GitKimUser closed this Dec 2, 2024
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.

Add location addresses data from Korea
2 participants