-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add global permissions #105
Conversation
ee76cd7
to
17da299
Compare
So a windows test seem to be failing but I am not sure if this related? |
b8e2846
to
d7d0998
Compare
So after looking at this again I have updated the PR with the following improvements
|
1d8f172
to
881ad03
Compare
@djspiewak Would it be possible to have a look at this? |
@djspiewak IMHO this pr has a huge value, and it's a feature that I'll love too |
So from what I can see @djspiewak doesn't seem to have capacity/time to properly take care of this project hence why I made #117 |
@eed3si9n You mind looking at this? |
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.
Overall LGTM. if this passes the CI, I'd merge?
Yup! I think the CI issues are unrelated but if needed I can look into it (I remember this project had windows related CI problems in general). |
881ad03
to
781b9f5
Compare
It was moved! Awesome :D |
781b9f5
to
e7395e8
Compare
@eed3si9n I will go ahead and merge this PR, just rebasing it now with the removal of the Windows in CI just to do a last minute sanity check to see it passes CI. |
PR adds the permissions section for generated workflow file.