nile icon indicating copy to clipboard operation
nile copied to clipboard

Refine project structure

Open ericnordelo opened this issue 2 years ago • 1 comments

@martriay @andrew-fleming I did a refactor to the project structure, trying to achieve better organization as a result of a call I had with Marto on Friday. I think it has a lot of room for improvement, but this is at least a first proposal (what is described is on this PR):

Informal specification:

project root (nile directory): Only for “core” wrappers (like cli and nre). The wrappers can import from core, but the opposite direction shouldn’t be allowed (core SHOULD NOT import from packages in root).

utils directory (module): For utilities that are helpful for but NOT specific to Nile (like felt_to_str or generic test helpers like assert_revert). For utilities specific to Nile logic (like accounts.py functions) use the common module inside core.

core directory: Core elements like Account (in account.py), including the commands and the common submodules. commands for command logic (like deploy or get-nonce), and common for utilities specific to Nile logic (like accounts.py functions). For generic utilities, use the utils module, sibling to core.

This module shouldn’t import anything from the root (except for utils, being generic utilities). This is why the deployments.py and the accounts.py modules have been moved to core (inside the common module because they are functions supporting other logic, not a core logic by itself, at the contrary of account.py, signer.py, or plugins.py, which have been kept on the root of core).

commands and common (submodules of core): ^^ the idea is presented above.

artifacts and base_project: remain with the same objective.

ericnordelo avatar Sep 19 '22 23:09 ericnordelo

This is much better!

I'm wondering if it makes sense to set account as a subdirectory (maybe inside commands?). That way, we can separate commands like compile and node from account-specific commands. The one thing I don't love about this suggestion is call_or_invoke because that sort of straddles both sides (since call doesn't have anything to do with accounts but invoke does).

That said, I think we should consider separating call_or_invoke. IMO it made sense before to lump them together because of the similar logic and we had the ability to invoke txs without an account. I suggest either:

  1. Have call.py and invoke.py as two separate modules. or
  2. Integrate the invoke logic directly into account.send and leave a simple call.py.
  • If it's too much logic in account.send, we can maybe include an internal _invoke method for the Account class.

#2 seems preferable to me^.

andrew-fleming avatar Oct 26 '22 19:10 andrew-fleming