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

changed the log and the package.json in order to add the new faststor… #1119

Open
wants to merge 4 commits into
base: 2.x
Choose a base branch
from

Conversation

jordi1215
Copy link

…e-cli plugin

What is the purpose of this pull request?

I am a winter intern and I developed a CLI plugin to facilitate the file linking process in the faststore framework. I was asked to integrate this plugin with Toolbelt.

What problem is this solving?

Adding the @vtex/faststore-cli to the dependencies so we can use this plugin

How should this be manually tested?

Run the faststore-cli commands on toolbelt

Screenshots or example usage

please refer to the readme file in @vtex/faststore-cli

Types of changes

  • [ x] New feature (non-breaking change which adds functionality)

Chores checklist

  • [ x] Update CHANGELOG.md

@jordi1215 jordi1215 requested a review from a team August 10, 2021 21:53
@jordi1215 jordi1215 requested a review from a team as a code owner August 10, 2021 21:53
Copy link
Contributor

@valentino-amadeus valentino-amadeus left a comment

Choose a reason for hiding this comment

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

We already have a link command... How is this going to work?

@jordi1215
Copy link
Author

jordi1215 commented Aug 23, 2021

We already have a link command... How is this going to work?

@valentino-amadeus I was able to change the implementation of my plugin so that all the commands will fall under the "faststore" namespace. For example, to run my link command, you would use vtex faststore link.

I was following an older version of instructions on how to implement a plugin. As far as I understand, with the newer version of toolbelt, you are able to search and install plugins that have the repository name "@vtex/cli-plugin-somehtingelse" right? In that case, there is no need for me to include my plugin as the dependency for this project? If this is truly the case then I can just close this pull request.

Thank you!

Copy link
Contributor

@valentino-amadeus valentino-amadeus left a comment

Choose a reason for hiding this comment

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

@valentino-amadeus I was able to change the implementation of my plugin so that all the commands will fall under the "faststore" namespace. For example, to run my link command, you would use vtex faststore link.

Nice!

you are able to search and install plugins that have the repository name "@vtex/cli-plugin-somehtingelse" right?

You can search the plugins in npm (is there another way? 👀) and install them from there.

there is no need for me to include my plugin as the dependency for this project?

There is no need, but if you choose not to do so, you may want to include this information in the (fastsore's?) documentation.

In case you choose to include this plugin as a dependency, it would be nice to update the commands list.

Comment on lines +12 to +13
### added
- [vtex faststore-cli] adding a new plugin called faststore-cli that facilitate linking for the faststore framework
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
### added
- [vtex faststore-cli] adding a new plugin called faststore-cli that facilitate linking for the faststore framework
### Added
- [vtex faststore-cli] Add `cli-plugin-faststore` plugin, that facilitates linking for the faststore framework

@viniagostini viniagostini removed the request for review from a team November 21, 2024 18:48
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