Workflow dispatch + dev branch tidy-up#882
Conversation
| { | ||
| "command": "codeQL.runRemoteQuery", | ||
| "when": "editorLangId == ql && resourceExtname == .ql" | ||
| "when": "config.codeQL.canary && editorLangId == ql && resourceExtname == .ql" |
There was a problem hiding this comment.
Have you verified this is working correctly?
There was a problem hiding this comment.
Yep, the commands are hidden in the right situations! Annoyingly, the authentication option itself is still visible from the side bar, even with the canary setting false...
That comes from the initialization step, which isn't feature-flagged.
Will have a look at tidying that up tomorrow! 🧹
There was a problem hiding this comment.
Yep, the commands are hidden in the right situations!
Nice!
Annoyingly, the authentication option itself is still visible from the side bar, even with the canary setting
false...
Hmmm...is it sufficient to wrap the call to Credentials.initialize in a check to see if canary is set?
Also, if someone changes from canary to not canary, it would be nice if they were logged out of github. You can set up a configuration listener and automatically log out if canary goes from true to false (similarly, you can automatically log in when the setting goes in the other direction).
There was a problem hiding this comment.
Also, if someone changes from canary to not canary, it would be nice if they were logged out of github. You can set up a configuration listener and automatically log out if canary goes from true to false (similarly, you can automatically log in when the setting goes in the other direction).
I did some more experimenting, and I don't know if it's possible to log a user out via the public authentication API if we're using a "built-in" AuthenticationProvider (i.e. GitHub or Microsoft)... I found a few comments where other people were also talking about the lack of a "logout" option and how the authentication providers are registered, so I think this might be a wider problem 🤔
For now, I've at least wrapped the call to Credentials.initialize in a canary check, so everything is fully hidden when it needs to be 🎉 Is that okay for now? I don't expect many users toggle the canary flag by accident, so it shouldn't have a noticeable impact!
There was a problem hiding this comment.
Oh well. Thanks for looking into this and apologies for suggesting something that isn't possible.
It's not a big deal since if someone is already logged in and in canary mode, then they are already aware of this feature and will have tried to use it. And it's sufficient to just hide the feature if canary is off. I'll take another look shortly.
|
I've given this a run-through locally and it worked well ❤️. It prompted me to remove the default values for the parameters on the controller workflow as they would be silently used if entries were missing from the |
See also: hackathon work from https://github.com/github/vscode-codeql/pull/701/files
d38b345 to
044c5b3
Compare
aeisenberg
left a comment
There was a problem hiding this comment.
Looks good. One small suggestion, which you can take or leave.
Two new "canary" commands: * GitHub authentication (from github#874) * Workflow dispatch (run remote query)
This branch is based on the
qc-developmentbranch, but (assuming the feature-flagging works) it should now be good to merge tomainwithout disrupting our external users.More details in internal linked issue and demo!
Checklist
(n/a—these features aren't useful to external users, so no need to publicize them)
@github/docs-content-codeqlhas been cc'd in all issues for UI or other user-facing changes made by this pull request.