-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat(msgraph): support advanced querying capabilities #9921
feat(msgraph): support advanced querying capabilities #9921
Conversation
🦋 Changeset detectedLatest commit: 759b32b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
0d4dc90
to
cd41668
Compare
cd41668
to
c3d5454
Compare
thanks this is a great addition :) |
c3d5454
to
c6e213c
Compare
@ehrnst feel free to review the changes, too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks OK with me. Not sure how the review-routine works with Backstage.
@ehrnst I believe if it looks ok to you, you can leave an approval. Maintainers will have a look, too. But that helps them if there was already another review from the community. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright, let's just update the changeset and then good to go!
@@ -0,0 +1,5 @@ | |||
--- | |||
'@backstage/plugin-catalog-backend-module-msgraph': patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be minor since it's a breaking change to a public interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you mean the change to the client.getUsers and client.getGroups functions, right @freben? Besides that queryMode is optional with backwards compatible default value applied at a low level.
If it is about these two functions, maybe a change of the position would reduce it again to "patch"? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would, yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freben I moved the new optional parameter to the end
@pjungermann just a small ping here :) |
c6e213c
to
639c7c1
Compare
Support advanced querying capabilities using the new config option ``` queryMode?: 'basic' | 'advanced' ``` which will set all required headers and query parameters. Closes: backstage#9795 Signed-off-by: Patrick Jungermann <Patrick.Jungermann@gmail.com>
639c7c1
to
759b32b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aight!
Support advanced querying capabilities
using the new config option
which will set all required headers and
query parameters.
Closes: #9795
Signed-off-by: Patrick Jungermann Patrick.Jungermann@gmail.com
Hey, I just made a Pull Request!
✔️ Checklist
Signed-off-by
line in the message. (more info)