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

docs: verbose plugin create command details #1445

Merged
merged 20 commits into from
Apr 5, 2023

Conversation

jthegedus
Copy link
Contributor

@jthegedus jthegedus commented Jan 22, 2023

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:

  • quickstart: how to get started with making a plugin
  • warn against anti-patterns
  • scripts overview: enumerate all scripts with descriptions in a single place. Clearly denote which are required scripts, recommended, and optional.
  • env var overview: enumerate all env vars used throughout all scripts
  • rewrite "required scripts" and "optional scripts" sections. include the following information:
    • env vars available to the script
    • asdf commands that invoke the script
    • call signature from asdf core, listing parameters
    • warn against anti-patterns we've seen in the particular script in the past (eg: 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

asdf-vm-before

After Picture

Red marks indicate the start and end of the page yet to be updated.

asdf-vm-after

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.

@jthegedus jthegedus self-assigned this Feb 19, 2023
@jthegedus jthegedus added the priority asdf core intend to resolve soon label Feb 19, 2023
@jthegedus jthegedus marked this pull request as ready for review April 5, 2023 13:20
@jthegedus jthegedus requested a review from a team as a code owner April 5, 2023 13:20
@jthegedus
Copy link
Contributor Author

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.

@jthegedus jthegedus changed the title docs: improve plugin create docs docs: verbose plugin create command details Apr 5, 2023
@jthegedus jthegedus merged commit 8108ca6 into master Apr 5, 2023
@jthegedus jthegedus deleted the docs-create-plugin-rewrite branch April 5, 2023 13:31
@hyperupcall
Copy link
Contributor

hyperupcall commented Apr 6, 2023

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.

@jthegedus
Copy link
Contributor Author

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.
Copy link
Member

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.
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

Is tac available everywhere?

Copy link
Contributor Author

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.
Copy link
Member

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.

@Stratus3D
Copy link
Member

This looks great @jthegedus ! Would you like me to create another documentation issue for the things I've commented on above?

@jthegedus
Copy link
Contributor Author

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.

@Stratus3D
Copy link
Member

I've created #1595 for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority asdf core intend to resolve soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants