-
-
Notifications
You must be signed in to change notification settings - Fork 802
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 "set" methods to ProtectedAsMock #1165
Add "set" methods to ProtectedAsMock #1165
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for keeping you waiting so long. I've been rather busy at work.
Good work! 👍 I'd be happy if you could make a few small changes (as per the review comments). Also, could you please add an entry to CHANGELOG.md
in the Unreleased section, for instance:
## Unreleased
+#### Added
+
+* `SetupSet`, `VerifySet` methods for `mock.Protected().As<>()` (@tonyhallett, #1165)
#### Fixed
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tonyhallett, thanks for making the requested changes!
I'm a little confused by the additional changes you've made. Can we please do the bugfixing of auto-mocking and virtual properties in one or more separate focused PRs... or is this somehow a necessary consequence of mixing your replacer class into the existing DuckReplacer
?
@tonyhallett, thanks for extracting the bug fix into a separate PR #1185. (Good work!) I've marked those review comments that were related to it as resolved, so you can now proceed with rebasing & cleaning up this PR; there are now only a few details remaining. I'll try my best to respond more quickly this time! |
@stakx You ok if I squash these commits into one ? |
@tonyhallett, sure, go ahead. Let me know once it's ready to be merged. |
Unfortunately this has highlighted my poor knowledge of git. I assume I need --rebase-merges.... |
@tonyhallett, if you want, I can do the squashing when I merge (GitHub's web UI has an option to squash-merge a PR). That would probably be easiest and quickest. If you want to do it yourself, for learning purposes, you can do so using an interactive rebase ( git checkout protectedasmock-set-methods
git rebase -i main # assuming that you forked `protectedasmock-set-methods` off the `main` branch
# in the editor that opens, change all except the first `pick` command to `squash`
# then save and exit the editor If you want to ensure that your branch is current with respect to this repository ( git remote add upstream https://github.com/moq/moq4.git
git checkout main
git pull upstream/main This should fast-forward your |
Agreed. Please do |
Done. 🚀 Thanks for your contribution! |
Fix #1164