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: Add info about dependencies for local development #1375

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

josmperez
Copy link
Contributor

Add info about working with dependencies for local development

Note: I'm aware that there are some ambiguities in the text that need to be resolved.

  • First, where are the environment variables added?
  • Second, for GF_INSTALL_PLUGINS=dependency and GF_INSTALL_PLUGINS=your-plugin-zip-via-url can we provide an example? Or use placeholder syntax for placeholders?

Fixes # #952

@josmperez josmperez added type/docs Changes only affect the documentation no-changelog Don't include in changelog and version calculations labels Dec 3, 2024
@josmperez josmperez self-assigned this Dec 3, 2024
@josmperez josmperez requested a review from a team as a code owner December 3, 2024 21:58
@josmperez josmperez requested a review from oshirohugo December 3, 2024 21:58
Copy link

github-actions bot commented Dec 3, 2024

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged. It will not be considered when calculating future versions of the npm packages and will not appear in the changelogs.

@academo
Copy link
Member

academo commented Dec 4, 2024

First, where are the environment variables added?

Environment variables are defined in your terminal environment, usually in your profile file, which can be bashrc, zshrc, and many others, or by exporting them before running commands export FOO=bar but it is much different if you are defining environment variables for a production server for example.

Working with environment variables is a well-known thing in development and IMO we should not be explaining that concept to developers, it'd be close to tell them how to install an editor or IDE, or how to install and open a terminal.

Furthermore, there are so many possible variations of how to do it that whatever we write most likely won't fit the case of the person reading, so it is simpler to say "set an environment variable" and let the developer do it its own way.

Copy link
Member

@academo academo left a comment

Choose a reason for hiding this comment

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

I am not sure i understand the idea behind this document, my main problem is that I don't know what do we mean by "dependency". can you maybe clarify that?

@sympatheticmoose
Copy link
Contributor

I am not sure i understand the idea behind this document, my main problem is that I don't know what do we mean by "dependency". can you maybe clarify that?

@academo Does the issue help or is it still unclear? #952

@academo
Copy link
Member

academo commented Dec 5, 2024

I am not sure i understand the idea behind this document, my main problem is that I don't know what do we mean by "dependency". can you maybe clarify that?

@academo Does the issue help or is it still unclear? #952

yes, that makes sense! I was almost sure we were talking about plugin depending on other plugins but in the current docs I could not see where it says that. If I skipped it and didn't read it, then maybe we should repeat it a couple times to not put things out of context?

It is easy to think of a plugin dependency as a library dependency, or grafana version dependency only

@sympatheticmoose
Copy link
Contributor

It is easy to think of a plugin dependency as a library dependency, or grafana version dependency only

💯

@josmperez
Copy link
Contributor Author

I am not sure i understand the idea behind this document, my main problem is that I don't know what do we mean by "dependency". can you maybe clarify that?

@academo Does the issue help or is it still unclear? #952

yes, that makes sense! I was almost sure we were talking about plugin depending on other plugins but in the current docs I could not see where it says that. If I skipped it and didn't read it, then maybe we should repeat it a couple times to not put things out of context?

It is easy to think of a plugin dependency as a library dependency, or grafana version dependency only

I've made a commit that clarifies that this additional section pertains specifically to plugins that depend on other plugins. Does this address your concerns? Do you see any other changes necessary?

@academo
Copy link
Member

academo commented Dec 9, 2024

I am not sure i understand the idea behind this document, my main problem is that I don't know what do we mean by "dependency". can you maybe clarify that?

@academo Does the issue help or is it still unclear? #952

yes, that makes sense! I was almost sure we were talking about plugin depending on other plugins but in the current docs I could not see where it says that. If I skipped it and didn't read it, then maybe we should repeat it a couple times to not put things out of context?
It is easy to think of a plugin dependency as a library dependency, or grafana version dependency only

I've made a commit that clarifies that this additional section pertains specifically to plugins that depend on other plugins. Does this address your concerns? Do you see any other changes necessary?

I left other comments that also require some changes or clarification.

@josmperez
Copy link
Contributor Author

First, where are the environment variables added?

Environment variables are defined in your terminal environment, usually in your profile file, which can be bashrc, zshrc, and many others, or by exporting them before running commands export FOO=bar but it is much different if you are defining environment variables for a production server for example.

Working with environment variables is a well-known thing in development and IMO we should not be explaining that concept to developers, it'd be close to tell them how to install an editor or IDE, or how to install and open a terminal.

Furthermore, there are so many possible variations of how to do it that whatever we write most likely won't fit the case of the person reading, so it is simpler to say "set an environment variable" and let the developer do it its own way.

Thanks for the thorough review. I've made corrections/changes in response to your comments. Please re-review.

@josmperez josmperez requested a review from academo December 11, 2024 22:26
@@ -439,3 +440,36 @@ Update the `scripts` in the `package.json` to use the extended Webpack configura
-"dev": "webpack -w -c ./.config/webpack/webpack.config.ts --env development",
+"dev": "webpack -w -c ./webpack.config.ts --env development",
```
## Add a dependency for local development
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Add a dependency for local development
## Add a plugin as a dependency for local development


Grafana’s plugin development environment is designed to allow for plugins to be dependent upon other plugins if necessary. Here’s how it operates:

Plugin distribution path:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Plugin distribution path:
Plugins location directory:


Plugin dependency management:

Although some validations occur on the server side, the plugin path itself doesn't include validation of other plugins your plugin may be dependent upon. You are responsible for ensuring that your plugin's dependencies are correctly managed.
Copy link
Member

Choose a reason for hiding this comment

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

This sentence is not clear, I don't really understand what is trying to tell me.

Is it telling me I am responsible of installing the plugin dependencies myself? or is it telling me that I am responsible of putting the dependencies in the correct path? or something else?

Copy link
Member

@academo academo left a comment

Choose a reason for hiding this comment

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

This looks better but, are we somewhere actually putting how to define the dependency in the plugin.json?

So far what we have tells the user they can define a dependency and (maybe?) they have to install it themselves, but how do they define the dependency?

@josmperez
Copy link
Contributor Author

This looks better but, are we somewhere actually putting how to define the dependency in the plugin.json?

So far what we have tells the user they can define a dependency and (maybe?) they have to install it themselves, but how do they define the dependency?

What's the fix for this? Can you make a suggestion that gives an example please?

@josmperez josmperez requested a review from academo December 16, 2024 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Don't include in changelog and version calculations type/docs Changes only affect the documentation
Projects
Status: 🔬 In review
Development

Successfully merging this pull request may close these issues.

3 participants