-
Notifications
You must be signed in to change notification settings - Fork 9
autogenerated-apimanagementapidiagnostic #87
Conversation
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.
@houkms Please address the comments.
list_resources.sh
Outdated
@@ -4,7 +4,7 @@ cd `dirname $0` | |||
for file in `ls ./specs`; do | |||
work_dir=./specs/$file | |||
if [ -d $work_dir ]; then | |||
if [ -n "$(git status -s $work_dir)" ]; | |||
if [ -n "$(git diff --name-only remotes/origin/master remotes/origin/$source_branch -- $work_dir)" ]; |
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.
What if we need to run it in local environment ? Can we add a fallback logic ?
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.
Add the fallback logic in script.
- We can run
make build SOURCE_BRANCH=branchname
, to generate codes for new added/modified resources in local branch - If
SOURCE_BRANCH
is not set, it uses current branch as default, and generates codes for new added/modified resources in current branch - For uncommitted changes, just uses
make build RESOURCE=resourcename
as before
azure-pipelines.yml
Outdated
@@ -79,6 +79,7 @@ steps: | |||
failOnStderr: true | |||
|
|||
- bash: | | |||
export source_branch=$(System.PullRequest.SourceBranch) |
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 would recommend pass the source_branch
as argument to Makefile instead of using environment variable.
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.
OK. Now the source_branch
is passed as argument.
Codes are modified according to the comments. |
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.
LGTM
No description provided.