Closed ahmetcihank closed 7 months ago
The committers listed above are authorized under a signed CLA.
Welcome @ahmetcihank! It looks like this is your first PR to knative-extensions/backstage-plugins 🎉
@ahmetcihank Thanks a lot for the PR!
It looks good, but I've added a few comments to increase the test coverage.
Also, can you make the required things for the CLA?
I actually noticed that there are some compilation errors.
They don't show up in GitHub actions. See https://github.com/knative-extensions/backstage-plugins/issues/45
Can you run yarn tsc
locally and check out the compilation errors?
Other than that, I would actually change the query
in the testcase definition to queries
and define multiple mock calls.
Adding an inline comment now.
Also, can you make the required things for the CLA?
Here is a short intro on what is CLA and how to do that, you may find this helpful! @ahmetcihank https://knative.dev/blog/articles/getting-started-blog-p2/#cla
/lgtm /approve
I know this took a bit long, thanks very much for your addressing comments patiently!
Can you fix the CLA issue? Then we can merge this one!
/lgtm /approve
I know this took a bit long, thanks very much for your addressing comments patiently!
Can you fix the CLA issue? Then we can merge this one!
CLA is fixed , It is my pleasure , to keep up with your comments.. Thanks a lot.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: ahmetcihank, aliok
The full list of commands accepted by this bot can be found here.
The pull request process is described here
in terms of #28 pagination is implemented and fixed tests accordingly , pagination limit is considered for 10000 for now. I tested in my local environment and it is working.