-
Notifications
You must be signed in to change notification settings - Fork 790
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
docs: verbose plugin create command details #1445
Conversation
d41905b
to
d7140c1
Compare
I am going to force merge this as this PR has been open far too long for what it is. Any feedback can be raised in a new PR. |
5dd39a0
to
692ae1c
Compare
Was #1028 intended to be closed? The new changes make things a lot cleaner and better and even though there are no "architecture diagrams" (i think that was mentioned somewhere), I think the changes are enough to warrant closing the issue. |
I think the crux of the issue with @1028 was that although the information is there (and has now been made even more clear), it does not actually guide a developer through the process of creating a plugin. I would like to at least have a page which walks through creating a plugin, similar to how the "Getting Started" guide goes through the process step by step. With that, I would then close #1028 |
|
||
## Scripts Overview | ||
|
||
The full list of scripts callable from asdf. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think it's important to note that these scripts are specifically callbacks that will be invoked at the right time by asdf. It might be good to emphasizing that somewhere here at the top just for clarity. These are "callback scripts invoked by asdf when needed".
|
||
## Environment Variables Overview | ||
|
||
The full list of Environment Variables used throughout all scripts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line seems to be at odds with lines 76 and 77. Maybe these three lines should be combined for clarity?
Must print a string with a space-separated list of versions. Example output would be the following: | ||
If versions are being pulled from releases page on a website it's recommended to | ||
leave the versions in the provided order as they are often already in the | ||
correct order. If they are in reverse order piping them through `tac` should |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is tac
available everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know tac
is part of the GNU coreutils, so no, not available everywhere, but somewhat common. This is a statement I carried across from the older docs.
There are definite improvements to be made here. I pushed this through to just get the updates out there. Iterative improvements are welcome.
If you want your plugin to work with asdf version 0.7._ and earlier and version 0.8._ and newer check for the presence of the `ASDF_DOWNLOAD_PATH` environment variable. If it is not set download the source code in the bin/install callback. If it is set assume the `bin/download` script already downloaded it. | ||
**Call signature from asdf core** | ||
|
||
No parameters provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: In Bash terminology, are values after the script name called "parameters" or "arguments" I would think they'd be called arguments in this case.
In a regular programming language, a function defines parameters it accepts, and when the function is invoked the caller passes it a set of corresponding arguments.
This looks great @jthegedus ! Would you like me to create another documentation issue for the things I've commented on above? |
Yes. I think if we enumerate the above points in another issue then it will be more visible that we're still wanting to make further improvements. Maybe someone from the community might even contribute more suggestions or PRs 🤩 😅 I agree with all the points you've raised. Thanks for reading this huge PR. |
I've created #1595 for this. |
Summary
This is an attempt to improve our Plugin Create documentation. #1028 raised some pieces of information we weren't really capturing.
The intent here was to be explicit about the information for each script independent of other parts of the page. An issue with the previous docs was that information about which env vars applied to which scripts and other caveats was spread throughout different sections of the docs and wasn't really clear to those who are wanting to understand just one script for their own plugin.
The state of the changes:
sort -V
usage)bin/list-all
(required)bin/download
(required)bin/install
(required)bin/latest-stable
(recommended)bin/help.overview
bin/help.deps
bin/help.config
bin/help.links
bin/list-bin-paths
bin/exec-env
bin/exec-path
bin/uninstall
bin/list-legacy-filenames
bin/parse-legacy-file
bin/post-plugin-add
bin/post-plugin-update
bin/pre-plugin-remove
Before Picture
After Picture
Red marks indicate the start and end of the page yet to be updated.
Having the entire list of scripts listed in the initial table and also the left pane makes this much more visually clear. Users will no longer have to read each part of the page in detail to get a grasp of the capabilities.
Other Information
This update is not a complete walkthrough of the process to create a plugin, that should probably be a blog post.
#1028 did mention the idea of capturing the architecture or flow visually. I decided that the maintenance of that was too much to add now. This change might be sufficient. If not, we can add flow diagrams in future updates.