Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(sdk)!: support errors in mock #2277
base: v1.6-dev-ugly
Are you sure you want to change the base?
feat(sdk)!: support errors in mock #2277
Changes from 51 commits
7a77ccf
b85f52e
6de23ab
fba6ef8
8af6776
8e278e3
5af8822
8269c1f
a9a96a0
920b50d
067db87
c684b3f
9da89a8
34ffdfb
60d220c
80d19e0
f78555e
4343b6e
50ad84c
e4e667a
4bdf0cd
5eac233
dfaaa4c
ac388a8
016450f
0738f76
438185f
170d5a9
e20a29d
c9c6c2f
e5e17a7
dace7fa
eca69f8
df706f4
1f954d5
034bdb2
1bc31a9
bb21a21
5a497ca
723b1c5
098f854
88dda82
cebc709
a0b2427
4f2bb21
100147b
38f073a
bd7499d
98bebee
65cdabd
e5f827c
905955e
40963dc
bf07db8
c1f972b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
We need to decided, either we keep mockable implementation in their modules, like address, or in mock, like execution types. I would say they should be in their modules otherwise the mock module will be a mess eventually.
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.
To be refactored in separate PR.
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.
From<(&RequestSettings, &RequestSettings)>
? ))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.
Function is not used, removed.
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.
Review bincode dependency configuration.
Setting
optional = false
for bincode while it's under themocks
feature seems inconsistent. This could cause bincode to be always included in the binary even when themocks
feature is disabled.Consider this fix to maintain consistency:
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.
optional = false
sounds a bit strangeThere 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.
@shumkov, you're correct. Setting
optional = false
here can causebincode
to be included in the binary even when themocks
feature is disabled. Changing it tooptional = true
ensures thatbincode
is only included when themocks
feature is enabled, maintaining consistency.(^‿^)/
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.
Removed
optional
, as it's in use even withoutmocks
feature.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.
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.
We use serde in dapi client
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.
It's all under
mocks
feature atm, we should introduce separateserde
feature. TBD in next PR.