+
Skip to content

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
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

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载