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

add remaining plugins, refactor exports and add test and lint commands #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jgbernalp
Copy link
Collaborator

@jgbernalp jgbernalp commented Jan 15, 2025

this huge PR 😅. Adds the following:

  • Migrates the remaining plugins:

    • BarChart-0.4.0
    • GaugeChart-0.4.0
    • MarkdownChart-0.4.0
    • PieChart-0.4.0
    • Prometheus-0.4.0
    • ScatterChart-0.4.0
    • StatChart-0.4.0
    • StaticListVariable-0.1.0
    • Table-0.4.0
    • TimeSeriesChart-0.4.0
    • TimeSeriesTable-0.4.0
    • TraceTable-0.4.0
    • TracingGanttChart-0.4.0
  • Changes the way the plugin is exported to match better the editing panels and options

  • bumps the version to 0.4.0 as the export changes are breaking changes

  • Adds test and lint commands

  • Fixes the issue of the panel and datasources editors

Review will be probably better on a small meeting.

This needs perses/perses#2557

@jgbernalp jgbernalp force-pushed the jgbernalp/add-remaining-plugins branch 7 times, most recently from 28a95f7 to 45dc2cb Compare January 15, 2025 14:49
Signed-off-by: Gabriel Bernal <gabrielbernalp@gmail.com>
@jgbernalp jgbernalp force-pushed the jgbernalp/add-remaining-plugins branch from 45dc2cb to ded75c6 Compare January 15, 2025 15:00
return execErr
}

if createGroupArchive {
if execErr := exec.Command("cp", path.Join(pluginName, archiveName), path.Join("./plugins-archive", archiveName)).Run(); execErr != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I forgot something, but here I don't get where the binary plugins-archive is coming from. I can admit I don't even get why we would need that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was just for convenience while developing locally to replace it in the Perses side. But we can remove it once we can load a development version of the plugin files from the /plugins/ endpoint in Perses.

@Nexucis
Copy link
Member

Nexucis commented Jan 16, 2025

increadible work @jgbernalp !!! 🤯 I just have a small comment and I was wondering when you are saying:

This needs perses/perses#2557

Does that mean we need to merge perses/perses#2557 before this one ? Which is odd for me as the CI is green here and red in the other one.

@Nexucis
Copy link
Member

Nexucis commented Jan 16, 2025

Review will be probably better on a small meeting.

I think we can move forward and merge this PR and make this meeting afterward.

@jgbernalp
Copy link
Collaborator Author

increadible work @jgbernalp !!! 🤯 I just have a small comment and I was wondering when you are saying:

This needs perses/perses#2557

Does that mean we need to merge perses/perses#2557 before this one ? Which is odd for me as the CI is green here and red in the other one.

Yes. Is green here because the test of the plugins in isolation are ok. But we don't have integration tests yet.

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.

2 participants