Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Rename ZeppelinOS/zos references, ZosConfig object and cli/lib packages #1077

Merged
merged 16 commits into from
Jul 12, 2019
Merged

Rename ZeppelinOS/zos references, ZosConfig object and cli/lib packages #1077

merged 16 commits into from
Jul 12, 2019

Conversation

jbcarpanelli
Copy link
Contributor

@jbcarpanelli jbcarpanelli commented Jul 8, 2019 •
edited
Loading

UPDATE: This PR renames:

  • ZosConfig to NetworkConfig
  • References to ZeppelinOS/zos in codebase.
  • References to ZeppelinOS/zos in md files.
  • References to EVM Packages (Now Ethereum Packages) in md files.
  • zos to @openzeppelin/cli
  • zos-lib to @openzeppelin/upgrades
  • Reference to both zos and zos-lib in package.json files (examples, packages)

@spalladino
Copy link
Contributor

Looking good so far!

@jbcarpanelli jbcarpanelli marked this pull request as ready for review July 11, 2019 21:29
@jbcarpanelli jbcarpanelli changed the title Rename strings and ZosConfig Rename ZeppelinOS/zos references, ZosConfig object and cli/lib packages Jul 11, 2019
@jbcarpanelli jbcarpanelli force-pushed the rename/remove-zos-references branch 2 times, most recently from e4481ff to 89a4fd8 Compare July 11, 2019 21:52
@jbcarpanelli
Copy link
Contributor Author

jbcarpanelli commented Jul 11, 2019 •
edited
Loading

Mmmh, it seems that a test is failing because an openzeppelin-zos contract is looking for the Initializable.sol contract from the zos-lib dependency, which does not exist.
We have two possiblities for making this work:

  1. Update openzeppelin-eth and import Initializable.sol from @openzeppelin/upgrades instead of from zos-lib
  2. manually install zos-lib in lib-complex.

I would prefer the latter, as I don't know how much effort is needed to rename all the imports in openzeppelin-eth. We can create an issue to track this and fix it later.

@spalladino
Copy link
Contributor

Let's discuss this with @frangio. I'm afraid this could have implications for our users as well.

spalladino
spalladino suggested changes Jul 12, 2019
View reviewed changes
Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

Thanks for all the work here, @jcarpanelli! I left a few comments, but looks solid!!

packages/cli/README.md Show resolved Hide resolved
packages/cli/package.json Outdated Show resolved Hide resolved
packages/cli/src/commands/unpack.ts Outdated Show resolved Hide resolved
packages/cli/src/models/Verifier.ts Outdated Show resolved Hide resolved
packages/lib/README.md Show resolved Hide resolved
packages/lib/package.json Outdated Show resolved Hide resolved
spalladino
spalladino approved these changes Jul 12, 2019
View reviewed changes
Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

Awesome! Gogogo!

@jbcarpanelli jbcarpanelli merged commit ee43478 into master Jul 12, 2019
@mergify mergify bot deleted the rename/remove-zos-references branch July 12, 2019 20:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@spalladino spalladino spalladino approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jbcarpanelli @spalladino