Repository navigation
Fix environment variable processing. (#406) - #407
Conversation
Search for static and non public members as well when overriding values.
|
Thanks for the report and proposed fix. I'll have to think about this a bit. It might be better just to make the GitHub token field public and non-static - I don't think there's a particular reason for it to be private - or static - but overriding private vars in other circumstances might not be something we want. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This report revealed a number of environment variables with the incorrect settings (private or static). I went through and instead of modifying environment helper, changed all properties we intend to be overridden by Environment Helper to be Public instance variables. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
After your changes I was back to always getting the "Missing GitHub access token. Please define GITHUB_ACCESS_TOKEN." error. I've added 2 commits making ENV_GITHUB_ACCESS_TOKEN a property and changed EnvironmentHelper to look for properties instead of fields. Now it can process the GITHUB_ACCESS_TOKEN environment variable again. |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 407 in repo microsoft/OSSGadget |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Thanks Radoslav Kolev (@radolin) for catching the needed change to grab properties. |
|
Radoslav Kolev (@radolin) one more thing, can you reply to the Contributor agreement bot? I can't accept the contribution otherwise. Julian (@jpinz) any comments/concerns here? |
Will, do, just need to get the blessing from legal for a new CLA. Might take a couple of days. |
Julian (jpinz)
left a comment
Julian (jpinz)
left a comment
There was a problem hiding this comment.
Assuming it works and everything, LGTM
|
Radoslav Kolev (@radolin) any update from your legal team? |
|
@microsoft-github-policy-service agree company="SUSE" |
|
All checks seem green now, sorry for the delay. |
Search for static and non public members as well when overriding values. Here's how the member is defined in GitHubHealthAlgorithm.cs:
private static string? ENV_GITHUB_ACCESS_TOKEN = null;
Searching for public members only misses it and it's never set from the environment variable.
I don't know if this is the right way to fix this, but at least it get's me one step further and I can actually run the oss-health tool for an npm project.