search

1Password / shell-plugins

Seamless authentication for every tool in your terminal.
https://developer.1password.com/docs/cli/shell-plugins/
MIT License
526 stars 172 forks source link

Improve UnnecessaryPluginNameDefined validation check to display only when the plugin field is defined #266

Open arunsathiya opened 1 year ago

arunsathiya commented 1 year ago

What does it concern?

The plugin SDK, design, or schema

Goal or desired behavior

This issue is a followup to the PR:

https://github.com/1Password/shell-plugins/pull/265

Assuming that PR is merged, the output of make plugin-name/validate command shows the validation check even when the plugin field is not defined in CredentialUsage block.

It'd be nice to show that warning only when the field is set. If the field is not set, that validation check can be completely skipped.

Current behavior

Even without the plugin name set, the validation check runs. It could be skipped in such cases.

image image

Relevant log output

No response

op CLI version

2.18.0

shell-plugins local ref

No response

hculea commented 1 year ago

I think that the description of the validation check is what makes it a bit confusing.

I would propose that, instead of including additional logic to check whether the plugin name is set in the CredentialUsage, to simply rephrase the description of this check to something more general: e.g. "Only credential usages referencing credentials from other plugins have the Plugin field set".

Additionally, this should be addressed in #265 as well, since these two are tightly coupled. But I'd love to first reach a conclusion in this thread: https://github.com/1Password/shell-plugins/pull/265#discussion_r1201949030 😄