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

Fix encoding to work with Cyrillic and Portuguese accentuation #57

Merged
merged 2 commits into from Jan 14, 2022

Conversation

breno12321
Copy link
Contributor

Context

As we know in the issue #56 some characters when read from the database are not being processing as valid UTF-8 Characters. So this fixes it to handle the full length of the data representation for them

Closes #56

@nitishm
Copy link
Owner

nitishm commented Jan 13, 2022

LGTM. @Shivam010 if there are no objections let's get this in?

Copy link
Collaborator

@Shivam010 Shivam010 left a comment

Choose a reason for hiding this comment

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

Yup, LGTM too.

Also, I don't exactly remember why I iterate over all the characters. @breno12321 Did you run all the test cases?

@Shivam010
Copy link
Collaborator

The GitHub action should have run for the pr, right? Why hasn't it run?

@breno12321
Copy link
Contributor Author

breno12321 commented Jan 13, 2022

I can further on add the GitHub Action but by the way the tests weren't passing before the change 👀 . Also I've known golang for 2 days so I'll have to study to fix them 🤷🏼‍♂️ hehe

image

@breno12321
Copy link
Contributor Author

@Shivam010 I created the #58 check if its this what you mean 👀

@Shivam010
Copy link
Collaborator

@breno12321 All the tests were successful earlier, have a look here https://github.com/nitishm/go-rejson/runs/2264211335?check_suite_focus=true
I have approved the request to run the tests in #58

@breno12321
Copy link
Contributor Author

Ah sorry my bad 🙃 I ran in my fork master and failed for some reason 🤔

@coveralls
Copy link

coveralls commented Jan 14, 2022

Pull Request Test Coverage Report for Build 1697933704

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0%) to 98.496%

Totals Coverage Status
Change from base Build 716585563: 0.0%
Covered Lines: 131
Relevant Lines: 133

💛 - Coveralls

@breno12321
Copy link
Contributor Author

@breno12321 All the tests were successful earlier, have a look here https://github.com/nitishm/go-rejson/runs/2264211335?check_suite_focus=true I have approved the request to run the tests in #58

Apparently was related to the rejson version @Shivam010 good to go ✅ 🕺🏼

@Shivam010
Copy link
Collaborator

Ok, tests are passing so, I am accepting the changes

@Shivam010 Shivam010 merged commit 9d882c8 into nitishm:master Jan 14, 2022
@breno12321 breno12321 deleted the FIX-ENCODING-JSONGET branch January 14, 2022 13:45
@breno12321
Copy link
Contributor Author

Thanks guys!! Lets continue the journey to learn go hehe

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.

Invalid data is read in JSONGet when a struct with Unicode strings is passed into JSONSet.
4 participants