Skip to content
This repository was archived by the owner on Mar 11, 2024. It is now read-only.

Conversation

@xhulz
Copy link
Contributor

@xhulz xhulz commented Nov 15, 2022

PR description

Ref: #260

Hey @kevinbluer @acuarica,

I'm working on this POC using truffle/environment as Nick suggested yesterday, and the things seems to be working fine. As I'm getting the provider from truffle/environment I've removed it from the SearchParams.

@kevinbluer, as we said, it's a draft PR in order to do some tests before taking some decision.

@michaeljohnbennett for visibility

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if documentation updates are required.

@xhulz xhulz requested review from acuarica, kevinbluer and michaeljohnbennett and removed request for kevinbluer and michaeljohnbennett November 15, 2022 17:29
@xhulz xhulz changed the title chore: truffle/environment has been installed Support network option in addition to providerUrl when debugging Nov 15, 2022
@acuarica
Copy link
Contributor

Hi @xhulz thanks for taking care of this. Maybe I'm missing something here, but if we remove the providerUrl/network option from the Debugger, how the user is going to select a network? Isn't the provider returned by @truffle/environment some sort of default value when no providerUrl/network is specified?

Sorry in advance if this is a premature comment, just wanted to make sure we are on right track.

@xhulz
Copy link
Contributor Author

xhulz commented Nov 15, 2022

Hey @acuarica, you are right, I forgot this detail.

I'll make these changes and push them again.

Thank you!

@xhulz
Copy link
Contributor Author

xhulz commented Nov 16, 2022

Hey @acuarica, @kevinbluer and @michaeljohnbennett

Now the debug is working using this logic: https://gist.github.com/gnidan/4579216299fac7a41c51e0e370e8ddc2 and based on this documentation: #260

Any question lmk

Thank you

@acuarica
Copy link
Contributor

Hey @xhulz, I've been doing some testing. Now I get an error when the debug network is not declared in the Truffle config file. In this instance, the mfork network is a Ganache instance up and running.

truffle-sample-project-1668615058425

@xhulz
Copy link
Contributor Author

xhulz commented Nov 16, 2022

Hey @acuarica,

Could you do another test on this one, please?

Hey @xhulz, I've been doing some testing. Now I get an error when the debug network is not declared in the Truffle config file. In this instance, the mfork network is a Ganache instance up and running.

@xhulz xhulz marked this pull request as ready for review November 16, 2022 18:52
@xhulz xhulz requested a review from acuarica November 16, 2022 18:53
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Left some changes what we might want to consider.

One more question, do you know why there are so many changes in yarn.lock (in writing down this here because GH does not allow me to put comments when diff is large).

@xhulz
Copy link
Contributor Author

xhulz commented Nov 17, 2022

Looking good. Left some changes what we might want to consider.

One more question, do you know why there are so many changes in yarn.lock (in writing down this here because GH does not allow me to put comments when diff is large).

Wow, weird. I'll get the last version. Thank you for let me know @acuarica :)

@xhulz
Copy link
Contributor Author

xhulz commented Nov 17, 2022

Hey @acuarica, I dunno why, but it seems the yarn.lock loose its styling. Check this out:

Screenshot 2022-11-17 at 08 57 05

i just rolled back the last version

Thank you for that!

@xhulz xhulz requested a review from acuarica November 17, 2022 09:14
Comment on lines -197 to -218
/**
* Retrieves the chain id of the `providerUrl`.
*
* @param providerUrl the url to get chain id from.
* @returns the chain id of the given `providerUrl`.
*/
private getNetworkId(providerUrl: string) {
const services = TreeManager.getItem(ItemType.LOCAL_SERVICE);

if (!services || !services.getChildren()) {
return undefined;
}

const projects = services.getChildren() as LocalProject[];
const project = projects.find((project) => {
const network = project.getChildren().at(0) as LocalNetworkNode;
return `${network.url.protocol}//${network.url.host}` === providerUrl;
});

return project ? getChainId(project.options.forkedNetwork) : undefined;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! By removing all the TreeManager related code within the Debugger we make it less brittle.

Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks again for this!

@xhulz
Copy link
Contributor Author

xhulz commented Nov 17, 2022

Hey @acuarica!

Thanks for helping me with this :) I appreciate it

Cheers!

@xhulz xhulz marked this pull request as draft November 18, 2022 18:27
@xhulz
Copy link
Contributor Author

xhulz commented Nov 18, 2022

Need to convert as draft since we've got some problems with the truffle/environment + webpack

@xhulz xhulz marked this pull request as ready for review November 21, 2022 13:19
@xhulz xhulz requested a review from acuarica November 21, 2022 13:19
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants