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

Add jsonSerialize as valid method name #132

Merged

Conversation

jtgoltz
Copy link

@jtgoltz jtgoltz commented Feb 3, 2021

Fixing issue: #117

@jtgoltz
Copy link
Author

jtgoltz commented Feb 4, 2021

I don't believe that Travis failed because of my code changes. Is there a known issue about some jobs failing?

@stronk7
Copy link
Member

stronk7 commented Feb 4, 2021

I don't believe that Travis failed because of my code changes. Is there a known issue about some jobs failing?

Don't worry, that's Travis CI hitting the new DockerHub pull limits. I've relaunched the failed job.

Soon, we'll be adding support for dockerhub login, it's really simple, see moodlehq/moodle-docker#150

And also, the new GitHub Actions are being added to this repo, see #131. And GHA jobs aren't exposed to those DockerHub limits (they settled an agreement).

Ciao :-)

@stronk7
Copy link
Member

stronk7 commented Feb 4, 2021

Tiny detail... I'd add the missing trailing comma "," at the end of the new line, like the original one had. It's a tiny details but helps to get simpler diffs when there are changes in many lines.

@jtgoltz
Copy link
Author

jtgoltz commented Feb 4, 2021

Tiny detail... I'd add the missing trailing comma "," at the end of the new line, like the original one had. It's a tiny details but helps to get simpler diffs when there are changes in many lines.

Such a tiny commit and there is still an error ;)

I pushed a new version

Copy link
Member

@stronk7 stronk7 left a comment

Choose a reason for hiding this comment

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

All ok, approving this, thanks @jtgoltz !

@stronk7 stronk7 merged commit 66f7ac5 into moodlehq:master Feb 5, 2021
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.

2 participants