search

bepronetwork / bepro-js

The repository contains a comprehensive documentation of the bepro.network ecosystem as a javascript framework. If you are new to BEPRO, you might want to check out the Website Overview or our public "Start Building" Page.
164 stars 94 forks source link

Change createNetwork to emit CreatedNetwork event #138

Closed bepro-bot closed 2 years ago

bepro-bot commented 2 years ago

Change createNetwork function on the smart-contract of NetworkFactory.sol to emit the CreatedNetwork event

Change the CreatedNetwork event to emit the creators address instead of amount

bepro-bot commented 2 years ago

@moshmage reviewed this with the following message:

looks good to me

moshmage commented 2 years ago

@jaymeschroeder it appears some conflicts have crept up, can you clear it up ?

jaymeschroeder commented 2 years ago

@jaymeschroeder it appears some conflicts have crept up, can you clear it up ?

I will fix it in a couple of hours when I have time. Thanks for the notice.

bepro-bot commented 2 years ago

@jaymeschroeder reviewed this with the following message:

It appears that the createNetwork function now emits the CreatedNetwork event from a change made by someone else, however it still emits the id, the address of the creator, and the amount. Should the amount still be removed? If not, then it looks fine without my merge now.

bepro-bot commented 2 years ago

@moshmage reviewed this with the following message:

@jaymeschroeder > Should the amount still be removed? If not, then it looks fine without my merge now. Yes, no need for the amount to be emitted - bounty is still in place.

bepro-bot commented 2 years ago

@jaymeschroeder reviewed this with the following message:

I've made the change to CreatedNetwork event however npm is throwing an error during the test. Seems unrelated?

jaymeschroeder commented 2 years ago

Should be ready to merge now.

moshmage commented 2 years ago

@jaymeschroeder sorry for the delay and the conflicts but if you have the time could you solve them so we merge this in ?

jaymeschroeder commented 2 years ago

@jaymeschroeder sorry for the delay and the conflicts but if you have the time could you solve them so we merge this in ?

No problem. Conflicts have been resolved.