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 test for resource JSON marshaling #2783

Merged
merged 1 commit into from May 17, 2023

Conversation

kldzj
Copy link
Contributor

@kldzj kldzj commented May 17, 2023

Is #55 still actively pursued?

Adds test for JSON marshalling specifically the RepositoryTag resource.

@gmlewis gmlewis changed the title Add test for resource JSON marshalling Add test for resource JSON marshaling May 17, 2023
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @kldzj !
LGTM.
Merging after the tests pass.

@kldzj
Copy link
Contributor Author

kldzj commented May 17, 2023

Perfect, for the next time, is it okay to accumulate a bunch of commits and create a bundled PR then or would you prefer them smaller?

@gmlewis
Copy link
Collaborator

gmlewis commented May 17, 2023

Perfect, for the next time, is it okay to accumulate a bunch of commits and create a bundled PR then or would you prefer them smaller?

For #55, feel free to make a larger PR with a bunch of commits... we always squash+merge in this repo anyway.

In general, if an issue is complex, multiple PRs are nice, but not necessary... but feel free to make a huge one for #55.

Also, you can wait to actually make the PR until you are ready for a code review.

Thanks, @kldzj !

@gmlewis gmlewis merged commit dff9dcc into google:master May 17, 2023
7 checks passed
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.

None yet

2 participants