This repository has been archived by the owner on Dec 13, 2019. 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.
[all] Introduction of AppRegistry and significant code improvements to facilitate it #163
[all] Introduction of AppRegistry and significant code improvements to facilitate it #163
Changes from 168 commits
eb57b4e
f595e92
ea3e638
ee87e1d
57a22af
d1ec21b
50c3b58
2c5f1c6
d476dda
e530a54
7204893
040c6cd
a4c8246
06eef49
ab6a721
74b0131
3c3efb9
5209268
97db455
a2b74fc
f01f3e1
ea5b857
ac98d9f
21cb39d
c1e811e
3587ced
4943c1c
371a949
ea1942c
2e9d1d0
d3df803
9c65232
f4c90eb
8a2ec40
199cda1
f477edb
7742628
672c909
ee0f1ea
f6b964d
be670e9
e7e5b83
5bb84c2
6cc7f5f
ef9599e
83ff327
9b5ba1d
386bc1f
6ff7e9d
52a4e33
45abda0
dacfb17
e888d8f
e59b079
578a7e2
401bcfe
66d6e05
c15821e
6610a43
64a649c
ba1f7b2
db3e6cf
bc7a13c
76cba94
68f9935
eb72fe0
85e7bf6
934e785
412e8e2
2efe499
4f06b36
bad5319
99a0c1f
a46535f
f4c1afb
a846fea
970cf7c
e80f659
581fd42
cdd8fd8
e8d0645
2ebe4f6
a54a8dc
e981fbd
f4f1f48
b846285
c2b6e30
f0a10b3
76cc5df
b4bcde8
95e878d
9dad2a7
e7370f3
292aef8
e6f3f7a
218f30e
3975b7c
911f235
4907627
c153b3f
e800771
16bc59a
ecce80f
093d578
ea105e7
491cea7
98729cb
52a6814
73b08c1
de4a5df
65022e7
1223a0b
cadbbb1
c2f5abe
2f11b37
7d73f57
4853e63
730a2ac
efdf6d9
fa4b7d8
1f617ad
7cbe3ab
76fdc9a
c84d736
7eb2c77
dc930e1
6ea67a4
cf34e0e
aab77d6
e3560fa
a6f2885
39c8717
e2a0518
89b838a
487716e
6828278
d90e743
999093b
ddd8513
8b1ccab
150bee9
5e21aee
6266e1e
2c14b11
92bf855
d9147dc
bea5f87
8e97134
75e3b08
a5453a6
ce4a2db
cfd97e2
1e8075a
64b8185
fce9bc6
0426c95
7b82b34
8314c17
99c3bc1
54793be
689b703
fba8b74
6c2f324
e936b21
0b3b846
b47b480
22e7d4d
a4d0d82
89ba64b
559f86e
907b397
0e6363c
a0f616d
6e861e4
93b78ea
53f264d
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.
do we not get top-level ganache anymore? how do some of the machine tests which run against an EVM work? can you reflect these in the README and circleci?
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've removed the test that hit the blockchain. I'd like to re-write the test and just hit against a new ganache uniquely used for that test suite. I'll update the README.
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.
Is the rewrite part of this PR? It was a pretty useful test
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 can add one back in but I think it would be better to do it in a new 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.
@snario has the README been updated to reflect this status?
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.
why can't they just be added back?
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.
@snario that's a pretty useful test (as @IIIIllllIIIIllllIIIIllllIIIIllllIIIIll mentioned) - let's either not remove it or make a tangible PR to add it back that's referenced here, so as to not lose track of it.
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 think if we add a suffix, it should be
AppDefinition
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.
Probably nicer if we don't require prefixes. In Solidity 0.5 we'll be able to enforce that the programmer inherits from some interface which we can call
interface AppDefinition { ... }
so it looks likecontract TicTacToe is AppDefinition { ... }