-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Paule96/warnings powershell #13122
Paule96/warnings powershell #13122
Conversation
Ping @mjroghelia |
ping @ericsciple can you review this? |
ping @sandermvanvliet can you review 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.
@paule96 added two comments for you. Otherwise looks good 👍
Tasks/PowerShellV2/powershell.ts
Outdated
} | ||
contents.push(script); | ||
// log with detail to avoid a warning output. | ||
tl.logDetail(uuidV4(),tl.loc('JS_FormattedCommand', script),null, 'command', 'command', 0); |
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 see you're using tl.logDetail
here where the rest of this is using console.log
. Perhpas for consistency this should remain the same or (and I suspect this is the case) if you only want this for a certain log level the console.log
should most likely be replaced.
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.
No I can't use console.log. See the comment above. This is the script that contains ##vso[task.logissue type=warning;]
. This will trigger always a warning. So to prevent this, I force with tl.logDetail that it isn't important.
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.
@sandermvanvliet so is it okay, like it is? Or is there a good method to escape ##vso[task.logissue type=warning;]
?
ping @sandermvanvliet |
@mjroghelia can you also review this? so, we can bring this to the customers :) |
Maybe we can merge this in hacktober? |
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.
@anatolybolshakov could someone on your team take a look at this one?
Checked out these changes on hosted images (Windows, Ubuntu, MacOS). Everything works as expected. |
all tasks done ;) @anatolybolshakov please review again 👍 |
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 agree with the suggestions from @anatolybolshakov, otherwise looks good!
Co-authored-by: Anatoly Bolshakov <[email protected]>
- fix version number in task.loc - fix casing of Azure DevOps in task.json
Thanks for the feedback. I fixed it :) please @anatolybolshakov give your final feedback :P |
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, thanks for contribution here!
Task name: PowerShellV2
Description: Track powershell warnings as Azure DevOps Warnings
Documentation changes required: Y
**Added unit tests:**N
Attached related issue: (Y/N) #13121
Checklist:
Result after this change