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.

Sign up for GitHub

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

Jump to bottom

fix: correctly use dynamic plugin config schemas #742

Merged
merged 1 commit into from
Nov 13, 2023
Merged

fix: correctly use dynamic plugin config schemas #742

merged 1 commit into from
Nov 13, 2023

Conversation

davidfestal
Copy link
Member

Description

Correctly use the dynamic plugins configuration schemasin the Showcase application

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@davidfestal davidfestal requested a review from a team as a code owner November 8, 2023 19:23
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Nov 8, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 496c26e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
backend Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2023

The image is available at: quay.io/janus-idp/backstage-showcase:pr-742!

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2023

The image is available at: quay.io/janus-idp/backstage-showcase:pr-742!

@davidfestal davidfestal changed the title fix: supprt dynamic plugin config schemas fix: correctly use dynamic plugin config schemas Nov 8, 2023
@davidfestal davidfestal added the cherry-pick-1.0.x This PR should be cherry-picked to the 1.0.x branch label Nov 8, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2023

The image is available at: quay.io/janus-idp/backstage-showcase:pr-742!

tumido
tumido reviewed Nov 9, 2023
View reviewed changes
dynamic-plugins/imports/package.json Outdated Show resolved Hide resolved
dynamic-plugins/quay.io plugin.txt Outdated Show resolved Hide resolved
@davidfestal davidfestal force-pushed the gather-dynamic-plugins-schemas branch 2 times, most recently from 4804d21 to 8f0decb Compare November 9, 2023 17:31
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 9, 2023

The image is available at: quay.io/janus-idp/backstage-showcase:pr-742!

schultzp2020
schultzp2020 approved these changes Nov 9, 2023
View reviewed changes
Copy link
Member

@schultzp2020 schultzp2020 left a comment

Choose a reason for hiding this comment

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

LGTM

@janus-idp Janus IDP
Copy link

janus-idp bot commented Nov 10, 2023 •
edited
Loading

backstage-showcase Tests on commit 218c2f6 finished with errors.
View test log

@github-actions GitHub Actions
Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-742!

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-742!

tumido
tumido requested changes Nov 10, 2023
View reviewed changes
Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

LGTM (while I hate we have to do this)

One change request:

I didn't get to review #752 in time and since it's actually this PR that demands janus-idp/cli bump to 1.4.0 can you please update it in the docs as well?

backstage-showcase/showcase-docs/dynamic-plugins.md

Line 188 in 183f894

"@janus-idp/cli": "1.3.3"

tumido
tumido requested changes Nov 10, 2023
View reviewed changes
Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

And one more: please bump as well for consistent (deduped) resolution:

backstage-showcase/packages/app/package.json

Line 62 in 183f894

"@janus-idp/cli": "1.3.3",

@tumido
Copy link
Member

tumido commented Nov 10, 2023 •
edited
Loading

And one last request. Running yarn export-dynamic on this PR produces diff in dynamic-plugins/wrappers/*/dist-dynamic/package.json for me:

-    "test": "backstage-cli package test --coverage",
+    "test": "backstage-cli package test --passWithNoTests --coverage",

can you please fix that?

I take that back. This seems to be a regression caused by: #739

Fixing this in a separate PR: #762

@davidfestal davidfestal mentioned this pull request Nov 13, 2023
Open
@davidfestal
Copy link
Member Author

@tumido

And one more: please bump as well for consistent (deduped) resolution:

backstage-showcase/packages/app/package.json

Line 62 in 183f894

"@janus-idp/cli": "1.3.3",

done

bump to 1.4.0 can you please update it in the docs as well?

done

@davidfestal
fix: correctly use dynamic plugin config schemas
496c26e 
Signed-off-by: David Festal <dfestal@redhat.com>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

tumido
tumido approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@tumido tumido left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@github-actions GitHub Actions
Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-742!

@davidfestal davidfestal merged commit e6df353 into janus-idp:main Nov 13, 2023
7 checks passed
@kadel kadel added the cherry-pick-OK This PR was successfully cherry-picked to the appropriate branch. label Nov 13, 2023
kadel pushed a commit to kadel/backstage-showcase that referenced this pull request Nov 13, 2023
@davidfestal @kadel
fix: correctly use dynamic plugin config schemas (janus-idp#742)
057b928 
Signed-off-by: David Festal <dfestal@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tumido tumido tumido approved these changes

@schultzp2020 schultzp2020 schultzp2020 approved these changes

Assignees
No one assigned
Labels
cherry-pick-1.0.x This PR should be cherry-picked to the 1.0.x branch cherry-pick-OK This PR was successfully cherry-picked to the appropriate branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Config schema aggregation is missing for dynamic plugins
4 participants
@davidfestal @tumido @schultzp2020 @kadel