Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: updated action args #50

Merged
merged 5 commits into from
Oct 24, 2023
Merged

Conversation

0xDiscotech
Copy link
Contributor

  • refactor: removed create packages and move the logic to create package

  • refactor: changed typing type to export type and changed abi for interface

  • feat: created copy solidity files function

  • refactor: removed sample interface on create readme function

CLOSES BES-87

* refactor: removed create packages and move the logic to create package

* refactor: changed typing type to export type and changed abi for interface

* feat: created copy solidity files function

* refactor: removed sample interface on create readme function
@0xDiscotech 0xDiscotech requested a review from gas1cent October 23, 2023 22:16
@0xDiscotech 0xDiscotech self-assigned this Oct 23, 2023
@linear
Copy link

linear bot commented Oct 23, 2023

BES-87 Refactor the action

  • Rename parameters
  • Review the defaults
  • Provide better descriptions in action.yml

Defaults:

out: 'out'
interfaces_dir: 'solidity/interfaces'
contracts_dir: 'solidity/contracts'
typing_type: 'interfaces'

Can we figure destination_dir from the parameters provided by the user? It's basically some directory + package_name + typing_type

Refactor:

  • rename typing_type
  • remove _dir from out_dir, interfaces_dir, contracts_dir, destination_dir
  • params in createPackages and createPackage are named differently, what's exportDir?
  • Make sure export / destination / out naming is applied consistently without mix ups. Maybe there is a better way to name those things?
  • Why is there createPackages at all? We transfer its logic to createPackage and reduce the complexity of the action.

@0xDiscotech
Copy link
Contributor Author

0xDiscotech commented Oct 23, 2023

Notes:

  • The logic to copy each interface or contract and its respective ABI was moved to a single function copySolidityFile since the logic was the same
  • The sampleInterfaceName was removed from createReadme function since I think it didn't provide much value and it forced to the copySolidityFile function to return a name which I think is not worth it. Used a hardcoded interface on the usage example inside the README - I can undo that change if it needed
  • The README will be updated in another PR

Comment on lines +35 to +39
// Copy the interfaces and their ABIs
copySolidityFiles(outDir, interfacesDir, destinationDir);

// Copy the contracts only if the export type is contracts
if (exportType === ExportType.CONTRACTS) copySolidityFiles(outDir, contractsDir, destinationDir);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Idea: Instead of exporting the interfaces ABIs as default, could we have an if for each exportType?
If the user wants to only export the interfaces he can set interfaces as argument. If he wants to export only the contracts he can set contracts. If he wants both, he could set all.
By this way, we wouldn't be forcing to export the interfaces if the user only wants to export contracts

Copy link
Member

Choose a reason for hiding this comment

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

👍🏻 added this to Linear to think about later.

action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
src/copySolidityFiles.ts Outdated Show resolved Hide resolved
src/createPackage.ts Outdated Show resolved Hide resolved
src/createPackage.ts Outdated Show resolved Hide resolved
src/createPackage.ts Outdated Show resolved Hide resolved
@gas1cent gas1cent marked this pull request as ready for review October 24, 2023 05:59
* refactor: exported copy solidity file as non default function

* fix: fixed typos over comments
@0xDiscotech 0xDiscotech requested a review from gas1cent October 24, 2023 15:29
@gas1cent gas1cent merged commit e3847c4 into main Oct 24, 2023
2 checks passed
@gas1cent gas1cent deleted the refactor/action-args-and-functions branch October 24, 2023 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants