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.

Sign up for GitHub

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

Jump to bottom

Add Github actions for Pull Requests #58

Merged
merged 4 commits into from
Jan 14, 2022
Merged

Add Github actions for Pull Requests #58

merged 4 commits into from
Jan 14, 2022

Conversation

breno12321
Copy link
Contributor

No description provided.

@breno12321 breno12321 mentioned this pull request Jan 13, 2022
Merged
@breno12321 breno12321 changed the title Add gh actions for pr Add Github actions for Pull Requests Jan 13, 2022
Shivam010
Shivam010 approved these changes Jan 13, 2022
View reviewed changes
.github/workflows/analysis.yml Show resolved Hide resolved
Shivam010
Shivam010 reviewed Jan 13, 2022
View reviewed changes
rjs/helper.go Outdated
Comment on lines 23 to 25
for _, s := range _lst {
by = append(by, byte(s))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup, I remembered the JSONType and some other methods did have some different customized output which was causing problems when used directly.

I'll look into it tomorrow, it's pressing now.

https://github.com/nitishm/go-rejson/runs/4808164372?check_suite_focus=true#step:5:263

Shivam010
Shivam010 requested changes Jan 13, 2022
View reviewed changes
Copy link
Collaborator

@Shivam010 Shivam010 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@breno12321
Let's continue the encoding issue in #57 and merge this here with only the workflow yaml changes.

Can you remove your changes from rjs/helper.go file, so that we can merge this?

@breno12321
Copy link
Contributor Author

Oh sorry I thought I didn't sent them together

@breno12321 breno12321 requested a review from Shivam010 January 14, 2022 01:08
@breno12321
Copy link
Contributor Author

@Shivam010 Uhm why it still don't pass even though its just github actions change 🤔

@coveralls
Copy link

coveralls commented Jan 14, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 1697907725

  • 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

@Shivam010
Copy link
Collaborator

It's the version issue, we are using the latest version of redislabs/rejson (i.e 2.0.6)

Redislabs have changed some command responses after 1.0.8 which is causing failures in our tests. I have run them locally with 1.0.8, they are working.

For now, limiting the support to 1.0.8 and merging changes

@Shivam010
Copy link
Collaborator

Ignoring the code-analysis / staticcheck too.

@Shivam010 Shivam010 merged commit d51ac5e into nitishm:master Jan 14, 2022
@breno12321 breno12321 deleted the ADD-GH-ACTIONS-FOR-PR branch January 14, 2022 13:39
@breno12321
Copy link
Contributor Author

Thanks <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shivam010 Shivam010 Shivam010 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@breno12321 @coveralls @Shivam010