Skip to content

Conversation

@joshLong145
Copy link
Contributor

  • Addition of further help information for plugin commands
  • Addition of hook and command info output on ignite plugin list

@joshLong145
Copy link
Contributor Author

Also thinking of adding an lastUpdated property to the Plugin struct to track the last time the binary was built.

@joshLong145 joshLong145 changed the title Feat/plugin list cmd updates feat: plugin list cmd updates Nov 11, 2022
@aljo242 aljo242 mentioned this pull request Nov 14, 2022
15 tasks
Copy link
Contributor

@tbruyelle tbruyelle left a comment

Choose a reason for hiding this comment

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

Nice addition, I was thinking of something like this because with the hook addition we need to show what the plugin is actually doing.

The current implementation breaks a little bit the concept of a list, because a plugin description takes multiple lines. For instance if my project has 2 plugins configured, the output of ignite plugin list is :

$ ignite plugin list
	ℹ️ example-plugin info
Path 						Status 		
/home/tom/src/ignite/plugins/plugin-example 	✅ Loaded 	

	💻 Commands
Use 		Under 	
ipfs 		ignite 	
shutdown 	ipfs 	

	🪝 Hooks
Name 		On Command 		
my-hook-serve 	ignite chain serve 	
my-hook-build 	ignite chain build 	

	ℹ️ my-new-plugin info
Path 						Status 		
/home/tom/src/ignite/plugins/my-new-plugin 	✅ Loaded 	

	💻 Commands
Use 		Under 	
my-new-plugin 	ignite 	

	🪝 Hooks
Name 	On Command 	

I think we should keep the table format for plugin list, with a reduced set of information, for instance we could use the nice icons you added :

$ ignite plugin list
Path 						Status 		
/home/tom/src/ignite/plugins/plugin-example 	✅ Loaded 2🪝 1💻 	
/home/tom/src/ignite/plugins/my-new-plugin 	✅ Loaded 1💻 	

Then to have the complete description, we need to add a new command ignite plugin describe <pluginPath> that would output everything like you did but for a single plugin.

WDYT?

@joshLong145
Copy link
Contributor Author

Nice addition, I was thinking of something like this because with the hook addition we need to show what the plugin is actually doing.

The current implementation breaks a little bit the concept of a list, because a plugin description takes multiple lines. For instance if my project has 2 plugins configured, the output of ignite plugin list is :

$ ignite plugin list
	ℹ️ example-plugin info
Path 						Status 		
/home/tom/src/ignite/plugins/plugin-example 	✅ Loaded 	

	💻 Commands
Use 		Under 	
ipfs 		ignite 	
shutdown 	ipfs 	

	🪝 Hooks
Name 		On Command 		
my-hook-serve 	ignite chain serve 	
my-hook-build 	ignite chain build 	

	ℹ️ my-new-plugin info
Path 						Status 		
/home/tom/src/ignite/plugins/my-new-plugin 	✅ Loaded 	

	💻 Commands
Use 		Under 	
my-new-plugin 	ignite 	

	🪝 Hooks
Name 	On Command 	

I think we should keep the table format for plugin list, with a reduced set of information, for instance we could use the nice icons you added :

$ ignite plugin list
Path 						Status 		
/home/tom/src/ignite/plugins/plugin-example 	✅ Loaded 2🪝 1💻 	
/home/tom/src/ignite/plugins/my-new-plugin 	✅ Loaded 1💻 	

Then to have the complete description, we need to add a new command ignite plugin describe <pluginPath> that would output everything like you did but for a single plugin.

WDYT?

I think this is a good path forward, I originally isolated the hooks and cmd table under respective flags. but I like this better. I will move the command and hook info to ignite plugin describe <pluginPath>

Base automatically changed from feat/plugin-cmd-flags to main November 15, 2022 21:10
@joshLong145
Copy link
Contributor Author

when I merged main into this branch there were suddenly many more file changes. tried to revert the merge of main but can't seem to get an accurate delta count

@tbruyelle
Copy link
Contributor

tbruyelle commented Nov 17, 2022

@joshLong145 I can't explain what happened, this is confusing. Probably related to the multiple merge between develop and rebase on main.

I think the best way to fix this is to rewrite the history. I was able to have the correct diff (I think) with the following commands from your branch :

$ git diff main -- ignite/cmd/plugin.go > ../plugin-list.patch  # create a patch that contains only the diff that we are interested in
$ git reset --hard main
$ git apply ../plugin-list.patch

Check if the diff is now correct (git diff main), and if you're agree with it, and if nothing is missing, commit and push force into your branch.

@joshLong145 joshLong145 force-pushed the feat/plugin-list-cmd-updates branch from fd725fb to 366e990 Compare November 17, 2022 20:20
@tbruyelle tbruyelle changed the title feat: plugin list cmd updates feat: plugin describe command Nov 18, 2022
@joshLong145
Copy link
Contributor Author

@tbruyelle @aljo242 I've reworked plugin list and plugin describe to use an instance of session let me know what you think. if this is not desired ill revert the commit.

tbruyelle
tbruyelle previously approved these changes Nov 21, 2022
@joshLong145 joshLong145 requested a review from aljo242 November 22, 2022 14:51
@aljo242 aljo242 merged commit fb64798 into main Nov 23, 2022
@aljo242 aljo242 deleted the feat/plugin-list-cmd-updates branch November 23, 2022 13:59
@aljo242 aljo242 added this to the v0.26.0 milestone Dec 13, 2022
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* patch apply from declare changes

* update to changelog

* moving of changelog update

* newline addition

* clean up

* removing error return

* moving to session based table output for list and declare plugin

* moved table output outside loop

* return errors and output empty lists

* updates per review comments

* review comments

Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: Thomas Bruyelle <[email protected]>
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.

4 participants