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

Migrate to create-plugin #148

Merged
merged 20 commits into from
Dec 12, 2022
Merged

Migrate to create-plugin #148

merged 20 commits into from
Dec 12, 2022

Conversation

iwysiu
Copy link
Contributor

@iwysiu iwysiu commented Nov 7, 2022 •
edited

Migrates from grafana-toolkit to create-plugin.

Fixes #141

@github-actions
Copy link

Backend code coverage report for PR #148
No changes

@github-actions
Copy link

Frontend code coverage report for PR #148
No changes

iwysiu
iwysiu commented Nov 15, 2022
View reviewed changes
.github/workflows/code-coverage.yml
@@ -6,5 +6,5 @@ jobs:
workflow-call:
uses: grafana/code-coverage/.github/workflows/code-coverage.yml@v0.1.10
with:
frontend-path-regexp: src
frontend-path-regexp:
Copy link
Contributor Author

@iwysiu iwysiu Nov 15, 2022 •
edited

Choose a reason for hiding this comment

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

Code coverage temporarily disabled. It doesn't currently get run on main, and getting code coverage to successfully run would require further changes. #154 is to enable it.

@iwysiu iwysiu marked this pull request as ready for review November 15, 2022 21:50
@iwysiu iwysiu requested a review from a team as a code owner November 15, 2022 21:50
@sarahzinger
Copy link
Member

Hey @iwysiu would you mind adding a description to this PR? I know this is mostly a chore, but it can be helpful both as a reader to put us in the right frame of mind when reading and also to future versions of us reading the code!

@iwysiu iwysiu changed the title run create-plugin Migrate to create-plugin Nov 21, 2022
@iwysiu
Copy link
Contributor Author

iwysiu commented Nov 21, 2022

Hey @iwysiu would you mind adding a description to this PR? I know this is mostly a chore, but it can be helpful both as a reader to put us in the right frame of mind when reading and also to future versions of us reading the code!

Of course! Sorry about that, I was planning to after I finished fighting with the code coverage and then I forgot.

sunker
sunker reviewed Nov 22, 2022
View reviewed changes
Copy link
Contributor

@sunker sunker left a comment

Choose a reason for hiding this comment

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

Not tested, but it looks good! Added a few minor comments.

docker-compose.yaml Outdated Show resolved Hide resolved
jest.config.js Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@iwysiu iwysiu requested a review from sunker November 22, 2022 16:35
kevinwcyu
kevinwcyu reviewed Nov 23, 2022
View reviewed changes
Copy link
Contributor

@kevinwcyu kevinwcyu left a comment •
edited

Choose a reason for hiding this comment

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

I'm getting an error when I select an existing X-Ray data source and when I try to add a new X-Ray data source.

Does anyone else see this?

Screen Shot 2022-11-23 at 12 27 53 PM

Could be related #149

@iwysiu
Copy link
Contributor Author

iwysiu commented Nov 23, 2022

I'm getting an error when I select an existing X-Ray data source and when I try to add a new X-Ray data source.

Does anyone else see this?

Screen Shot 2022-11-23 at 12 27 53 PM

Could be related #149

@kevinwcyu Yeah, I'm also seeing that. It looks like the frontend isn't finding the datasource for some reason?
Screen Shot 2022-11-23 at 4 51 07 PM

I tried it with the migrated timestream pr and it wasn't erroring, so it may be something specific about x-ray. In which case, maybe it is the same bug.

kevinwcyu
kevinwcyu reviewed Nov 29, 2022
View reviewed changes
docker-compose.yaml Outdated Show resolved Hide resolved
@iwysiu @kevinwcyu
Update docker-compose.yaml
8b07e55 
Co-authored-by: Kevin Yu <kevinwcyu@users.noreply.github.com>
kevinwcyu
kevinwcyu reviewed Nov 29, 2022
View reviewed changes
.config/webpack/webpack.config.ts Outdated
'@grafana/ui',
'@grafana/runtime',
'@grafana/data',
'tslib',
Copy link
Contributor

Choose a reason for hiding this comment

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

@iwysiu, I think you can remove tslib from this list and it should stop having the 404 error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, it worked!

@iwysiu iwysiu merged commit 0aed083 into main Dec 12, 2022
@iwysiu iwysiu deleted the iwysiu/141 branch December 12, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinwcyu kevinwcyu kevinwcyu approved these changes

@fridgepoet fridgepoet Awaiting requested review from fridgepoet fridgepoet is a code owner automatically assigned from grafana/aws-datasources

@sunker sunker Awaiting requested review from sunker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

X-Ray: Migrate to create-plugin
4 participants
@iwysiu @sarahzinger @sunker @kevinwcyu