NEVISACCESSAPP-5452: Add logic to retrieve credentials from local.pr… #11
Conversation
…perties - Fix the rest of the retrieval order so system-wide values can be overridden per project
|
|
||
| if (project.hasProperty(name)) { | ||
| return project.getProperty(name) | ||
| def env = System.getenv(name) |
There was a problem hiding this comment.
I think env variable should be checked first as it was before. The CI jobs tries to override the project properties using env variables. If it is checked last then always the project variables will be used those committed in related code base in GitHub repository.
There was a problem hiding this comment.
This means the change warrants a team discussion. We did this originally for the React Native example app and we, together with @daniel-toth-leeder and @tamas-toth were surprised why environmental variables were checked first and thought it is a mistake. Now it seems like we know the reason and it is problematic...
We developers should AFAIK have the GITHUB and CLOUDSMITH credentials set as environmental variables, but in the gradle scripts (sometimes) the variables with GITHUB in their names are used for CLOUDSMITH (e.g. in the public example apps where we don't want the name "CLOUDSMITH" appear in the public build scripts). If the environmental variables are checked first, there is no way to override them per-project...
Well, unless we check local properties before environmental variables, which could solve this problem after this change with the logic to retrieve them from there - maybe this will be the solution to this.
There was a problem hiding this comment.
Until we decide this, I will convert this and other related PRs to Drafts - unfortunately, some were already merged.
|
I closed this issue until we figure out the order we would ultimately use, until then I created a new PR for just adding retrieval logic from local.properties: #12 |
balazs-gerlei
deleted the
feature/NEVISACCESSAPP-5452-retrieve-credentials-from-local-properties
branch
…operties