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(sdk):renamed sdk package to api client #324

Merged
merged 1 commit into from
Oct 26, 2020
Merged

refactor(sdk):renamed sdk package to api client #324

merged 1 commit into from
Oct 26, 2020

Conversation

yash2189
Copy link
Contributor

Fixes #314

Previously there was a package named cactus-sdk which had ambiguity within the project since it was referred to as an SDK of SDK's. This PR renames the cactus-sdk to cactus-api-client and changes the documentation , files supporting the same.

What has been updated ?
1 . In the package.json file, the properties: name, main, browser, browserMinified, mainMinified properties are all updated
2. The references in all the other packages are also updated to import the new package instead of the old one
3. Documentation is updated (README.md)

The changes for the updated directory can be found here

@petermetz
Copy link
Contributor

@yash2189 Thank you for the PR! Two things:

  1. The build has to work, but you've got compilation errors in some of the code still.
  2. The commit message is invalid, I think you are missing a space character after the colon.

Other than these (and with the assumption that the CI will pass) it looks good to me.

@yash2189
Copy link
Contributor Author

yash2189 commented Oct 24, 2020

@petermetz Thanks for the review. The errors a related to npm linting and lerna . Could you please tell me about the errors as I'm quite confused.

@petermetz
Copy link
Contributor

@petermetz Thanks for the review. The errors a related to npm linting and lerna . Could you please tell me about the errors as I'm quite confused.

@yash2189 I think you forgot to replace all the package imports from 'sdk' to 'api-client' in the code of the other packages that depend on @hyperledger/cactus-sdk.

See: https://travis-ci.org/github/hyperledger/cactus/jobs/738502564#L592

@petermetz
Copy link
Contributor

@yash2189 Looks like now you added one too many spaces. The one on the left from the parentheses is tripping the commit message linter if I'm guessing it right.
refactor (ask): renamed sdk package to api client should be refactor(sdk): renamed sdk package to api client
(also I assume the ask part is meant to be sdk

@yash2189
Copy link
Contributor Author

@yash2189 Looks like now you added one too many spaces. The one on the left from the parentheses is tripping the commit message linter if I'm guessing it right.
refactor (ask): renamed sdk package to api client should be refactor(sdk): renamed sdk package to api client
(also I assume the ask part is meant to be sdk

sorry typo , I'm doing it again

@yash2189
Copy link
Contributor Author

yash2189 commented Oct 25, 2020

Hi @petermetz , All the 3 checks have been passed , I have made the changes.
Please tell me whether I need to squash the commits ?
Thanks.

@petermetz
Copy link
Contributor

Hi @petermetz , All the 3 checks have been passed , I have made the changes.
Please tell me whether I need to squash the commits ?
Thanks.

Awesome, thank you so much @yash2189 !
Yes, please do squash the commits into a single one and then it LGTM!

Signed-off-by: yash2189 <yashajgaonkar24@gmail.com>
@yash2189
Copy link
Contributor Author

yash2189 commented Oct 25, 2020

@petermetz Done! Thanks for guiding me throughout the process. :)

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@takeutak takeutak left a comment

Choose a reason for hiding this comment

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

LGTM

@petermetz petermetz merged commit b70266b into hyperledger-cacti:master Oct 26, 2020
petermetz added a commit to petermetz/cacti that referenced this pull request Oct 26, 2020
This commit updates references of cactus-sdk that were missed by the earlier pull request:
hyperledger-cacti#324

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>

hyperledger-cacti#314
petermetz added a commit that referenced this pull request Oct 27, 2020
This commit updates references of cactus-sdk that were missed by the earlier pull request:
#324

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>

#314
ryjones pushed a commit that referenced this pull request Feb 1, 2023
IIN Agent Auto Sync Bug and Deploy Workflow improvement
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.

refactor(sdk): rename sdk package to api-client
3 participants