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

Stdlib/lib/dependency rename to package #240

Merged
merged 2 commits into from
Oct 17, 2018
Merged

Conversation

jbcarpanelli
Copy link
Contributor

@jbcarpanelli jbcarpanelli commented Oct 12, 2018

Renamed stdlib/lib/dependency to package in messages, labels, descriptions and json attributes.

Fixes #238.
Related to #272.

@jbcarpanelli jbcarpanelli added the status:to-review Awaiting review label Oct 12, 2018
@spalladino spalladino self-assigned this Oct 12, 2018
@spalladino
Copy link
Contributor

Awesome work Jota! I didn't even know there were still references to stdlib around the codebase.

The only thing I'm not sold on is removing the term dependencies. In many cases I think it makes sense to keep them, especially since using packages arbitrarily is confusing with the project package itself. For example, the zos.network.json file will now have a package entry (for the project package) and a packages one (for the dependencies). Also, using package: true and isPackage: true in the JSON files feels awkward, especially having a package: { address: '0x...' } entry as well.

As for the commands, I'd try alternating between dependencies and packages depending on the context, for instance: --deploy-packages => --deploy-dependencies.

@ElOpio or @maraoz could you chime in? I don't want to change anything you guys have already agreed upon regarding naming, but I'd appreciate if you could review it.

@maraoz
Copy link
Contributor

maraoz commented Oct 12, 2018

Agree 100% with @spalladino on the "dependencies" vs "packages" comment. Actually I had this concern in my mind when we decided to rename "libraries" to "packages", happy to see it addressed.

@jbcarpanelli jbcarpanelli added status:in-progress Under development, do not merge this PR and removed status:to-review Awaiting review labels Oct 15, 2018
Fix unit tests

Modify option on integration test command call

Rename dependencies to packages in vouching

Change and revert some names
Change 'package' to 'dependencies' when necessary
@jbcarpanelli
Copy link
Contributor Author

jbcarpanelli commented Oct 16, 2018

Note: I squashed my previous commits to avoid dealing with conflicts in each commit while rebasing on top of master, sorry for that.
Please let me know if I'm missing anything

@jbcarpanelli jbcarpanelli added status:to-review Awaiting review and removed status:in-progress Under development, do not merge this PR labels Oct 16, 2018
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.

Looks great! Left a few comments only.

const signature = `${name} [libraries...]`
const description = 'unlinks libraries from the project. Provide a list of whitespace-separated library names'
const signature = `${name} [packages...]`
const description = 'unlinks packages from the project. Provide a list of whitespace-separated package names'
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the same naming in both link and unlink (I'd go for dependencies)

const signature = `${name} [libraries...]`
const description = 'links project with a list of libraries located in each library npm package'
const signature = `${name} [dependencies...]`
const description = 'links project with a list of dependencies located in its npm package'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const description = 'links project with a list of dependencies located in its npm package'
const description = 'links project with a list of dependencies each located in its npm package'

Copy link
Contributor

Choose a reason for hiding this comment

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

Look ma, I'm using Github suggestions!!

@@ -5,7 +5,7 @@ import runWithTruffle from '../utils/runWithTruffle'

const name = 'freeze'
const signature = name
const description = 'freeze current release version of your stdlib project'
const description = 'freeze current release version of your package project'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const description = 'freeze current release version of your package project'
const description = 'freeze current release version of your published project'

Now we can freeze any kind of project, as long as its published.

@spalladino spalladino removed the status:to-review Awaiting review label Oct 17, 2018
@spalladino spalladino merged commit 9979655 into master Oct 17, 2018
@spalladino spalladino deleted the rename-lib-to-package branch October 17, 2018 16:13
facuspagnuolo pushed a commit that referenced this pull request Oct 23, 2018
* [CI skip] First draft of a new section describing the unstructured storage proxy pattern.

* [CI skip] Finished first draft of Proxy section.

* [CI skip] Merged some items in advanced.md to proxies.md

* [CI skip] Assimilated some of elopio's reviews

* [CI skip] Minor spelling fixes

* [CI skip] Added a few diagrams to the proxies section.

* Modified advanced section about json files to account for light mode.

* Removed (or names!)

* [CI skip] updated advanced section, jsons sub-section about the --full option.

* [CI skip] Typo fix explicitely > explicitly

* Update dependencies version on push (#279)

* Reupload contract if related lib has changed (#280)

* Add vouching kovan deploy

* Use version of dependency for identifying proxies in network json file (#281)

Changes both create and update commands to retrieve the package version
of either the project package or the dependency package when annotating
the proxy version in the network json file.

* Link command now successfully runs npm install again (#278)

* Check if project is published before freeze (#285)

* Check if project is published before freeze

* Check if package exists instead of if app exists

* docs: remove basil and crafty demos (#277)

The demos are outdated. For 2.0, they should probably be replaced with
simpler examples.

Fixes: #96

[CI skip]

* Stdlib/lib/dependency rename to package (#240)

* Rename stdlib/dependency to package

Fix unit tests

Modify option on integration test command call

Rename dependencies to packages in vouching

Change and revert some names
Change 'package' to 'dependencies' when necessary

* Implement suggestions from spalladino

* Fix storage layout parsing when there are complex types (#286)

Given a contract with a mapping or array (not struct), the check for
whether it contained structs or enums failed if there was no variable
with the same type as the value type of the mapping or array. For
instance, a contract with a variable of type `array(string)` and another
`string` would work, but a contract with just `array(string)` would
fail.

* Change status fix/fetch error message (#289)

* Preserve error stack trace when using ScriptError (#287)

* Update and unify package json files structures (#282)

* Check if package is frozen before uploading solidity libs (#284)

Previously the check was done only when uploading contracts, and caused
the solidity lib upload to fail with an EVM revert.

* Cleanup the READMEs (#276)

* Clean the main README

Fixes #232

* Added links to the blog and twitter

* Added links to the blog and twitter

* Update the docs README with suggestions by @facuspagnuolo

* Fix docs readme clone instructions

* Update to v2.0.0-rc.0

* Update CLI package lock

* Use latest versions of TPL-eth and OZ-eth (#295)

* Update to v2.0.0

* Fix vouching deployment script

* Update CLI package lock

* Update TPL-eth and OZ-eth to latest released versions

* Add zos files to vouching package files list

* Applied @facuspagnuolo's suggestion about publish

[CI skip]

* docs: restructure the docs webiste

Fixes: zeppelinos/zos-docs#94

[CI skip]

* Remove the duplicated proxies

* Fix the new in zOS 2 page.

[CI skip]

* [CI skip]

* correction for consistency

* fix small mistakes

* fix small mistakes
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.

3 participants