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

Restyle Monorepo #260

Closed
wants to merge 13 commits into from
Closed

Restyle Monorepo #260

wants to merge 13 commits into from

Conversation

restyled-io[bot]
Copy link

@restyled-io restyled-io bot commented Jun 25, 2019

A duplicate of #254 with additional commits that automatically address
incorrect style, created by Restyled.

Since the original Pull Request was opened as a fork in a contributor's
repository, we are unable to create a Pull Request branching from it with only
the style fixes.

The following Restylers made fixes:

  • shfmt
  • autopep8

To incorporate these changes, you can either:

  1. Merge this Pull Request instead of the original, or

  2. Ask your contributor to locally incorporate these commits and push them to
    the original Pull Request

    Expand for example instructions
    ```console
    git remote add upstream https://github.com/singnet/snet-cli.git
    git fetch upstream pull/<this PR number>/head
    git merge --ff-only FETCH_HEAD
    git push
    ```
    

NOTE: As work continues on the original Pull Request, this process will
re-run and update (force-push) this Pull Request with updated style fixes as
necessary. If the style is fixed manually at any point (i.e. this process finds
no fixes to make), this Pull Request will be closed automatically.

Sorry if this was unexpected. To disable it, see our documentation.

Reorganized sdk and cli to be two different packages in the same
repository

Moved sdk to cli repo

Rewrote SDK to match JS SDK implementation

CircleCI fix
…el with interceptor in service_client instance
optional parameter

Added call allowance to payment channel strategy as an optional
parameter

Split .gitignore to have a top level one and separate ones for each
package

Same for LICENSE

Updated CLI README, added top-level README (sdk-specific is still in
progress)
this monorepo is organized as a PEP 420 compliant namespace
(https://www.python.org/dev/peps/pep-0420/).

The sdk package has been restructured to conform to the PEP 420
namespace package specification and it's under the "snet" namespace.

The cli package is both a regular package and a namespace package.

setup.py for the monorepo itself installs the dependencies for all
packages in the repository.

Each package and the overarching monorepo also have "requirements.txt"
files for local installations and CI enviroments: by installing via
"requirements.txt", the dependencies listed in "setup.py" are only
pulled and installed from PyPI if a local version is not already
available. This allows to get dependencies within the monorepo to be
installed from the local filesystem.

The whole package is called "snet" and gives the name to the namespace.
The subpackages are "snet.sdk", "snet.snet_cli" and snet_cli (since the
CLI package is both within the namespace and a regular package).
Going forward, all of the code in the "snet_cli" package can be migrated
to the namespace package: both were kept momentarily for compatibility.

In this scenario, everything inside the "snet.sdk" package has
visibility on the contents of the "snet.snet_cli" package and
vice-versa.
Everything inside the "snet_cli" regular package has visibility on the
contents of the "snet.snet_cli" package due to the regular Python import
mechanics, but it has no visibility on the contents of the "snet.sdk"
package (and vice-versa).
So, the shared code between the SDK and the CLI goes under the
"snet.snet_cli" package which is at "packages/snet_cli/snet/snet_cli".

Code and assets deduplication:

thanks to the new structure, everything under "resources" has been moved
to the "snet.snet_cli" package and is now shared between the SDK and
CLI. Specifically, this includes the json artifacts from the compilation
and deployment of the smart contracts, the raw ".proto" files to
interact with the daemon and the compiled python client libraries.

Changes:

since the CLI is both a namespace package and a regular package, changes
to the code have been kept to a minimum.
The only change of note is that state service does not compile its own
".proto" files at runtime inside the user's home directory anymore but
they are imported from the "resources" directory under the
"snet.snet_cli" namespace package, where they are compiled during setup.
getting abi for OpenChannel event from the contract object using web3
function

passing instance of payment channel management strategy to service
client
@restyled-io restyled-io bot mentioned this pull request Jun 25, 2019
@vforvalerio87
Copy link
Contributor

Mother of God.

@restyled-io restyled-io bot force-pushed the pull-254-restyled branch from d356631 to 6211852 Compare June 25, 2019 03:04
@arturgontijo
Copy link
Contributor

It didn't:

  • Break the longs in-line imports
  • Remove unused imports
  • Remove the "redundant parenthesis"

But this is very useful!

@astroseger
Copy link
Collaborator

Hmm.. It adds a lot of newlines (probably because of 80 character rule), which, make code a little bit less nice.

For example it convert (mpe_client_command.py)

server_state  = self._get_channel_state_from_server(grpc_channel, channel_id)

to

        server_state = self._get_channel_state_from_server(
            grpc_channel, channel_id)

I'm not sure that it is a big problem though...

@raamb
Copy link
Member

raamb commented Jun 26, 2019

Given the magnitude of the base PR, I propose that we close this PR. Lets use restyled in all subsequent managable PRs.

Hmm.. It adds a lot of newlines (probably because of 80 character rule), which, make code a little bit less nice.

I am with you on this, prefer a single line.

It didn't:

  • Break the longs in-line imports
  • Remove unused imports
  • Remove the "redundant parenthesis"

But this is very useful!

Let me check how much of this can be achieved by configuring the bot.

@raamb
Copy link
Member

raamb commented Jun 28, 2019

Closing this PR. Will enable restyle now

@raamb raamb closed this Jun 28, 2019
@kiruxaspb kiruxaspb deleted the pull-254-restyled branch October 4, 2024 09:29
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.

5 participants