This repository has been archived by the owner on Feb 3, 2023. It is now read-only.
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.
Get links Count #1568
Get links Count #1568
Changes from 35 commits
a1e1eb5
35be08a
36003f0
2f1e016
bac4799
b16a4fc
4ac5a43
29bcc94
aa9bfaf
550e21e
8dcaaa3
2687c7a
656afcb
e24601e
0f77919
85c5d4f
5380335
504364a
cbd0ec3
e34657c
b8765df
a24cf72
408107c
89ebdeb
169cfdd
6a2f87c
f58c576
26913eb
5934479
58533f0
3e0ed46
ee969fd
7c2bb8e
19be4fa
ee932bd
25fc7ed
ba7fbb8
e91ab6c
51eb5a5
d20bf0c
861bdc5
054dd9a
e889a95
3f6352b
7643133
6438ee8
eca611a
1a46d86
0f2d83b
168f4e2
18e53f7
59554e3
1617097
bc566d4
50a8c4a
247338c
83c339b
6d0f1fc
f8454ec
df3700c
193441a
97d67fa
04b6285
9982399
d369c77
9645f52
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.
I changed this just to trigger the CI again
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.
also need to add this test into app_spec_proc
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.
I thought app_spec_proc copies over this from test.js?
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.
These assertions actually fail: https://circleci.com/gh/holochain/holochain-rust/31280?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
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 discussed merging Get* / RespondGet* to Query / RespondQuery. That would make this PR already smaller as we would need all that cruft including a new future etc.
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.
I thought about this and just used GetLinks with an enum because one thing I was worried about was the disconnect between what happens on the from the request point of view and the sever point of view. The reason why GetLinksCount was implemented explicitly because following it down the workflow is easy in terms of "new developers" and someone who isn't just new to the system. But I just used an enum as a parameter instead because doesn't over complicate what is happening there.
RespondGet merges and RespondQuery I think is something that should come into play as a later PR imo because some of this stuff can really propagate through a lot of changes making the scope much bigger as it goes.
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.
This could have to be more comonized but having explicit enums like this tells you exactly what it is doing.