Skip to content
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.

Sign up for GitHub

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

Jump to bottom

VSBuildV1 - added ability to disable default logger #14331

Merged
merged 21 commits into from
Feb 12, 2021
Merged

VSBuildV1 - added ability to disable default logger #14331

merged 21 commits into from
Feb 12, 2021

Conversation

anatolybolshakov
Copy link
Contributor

@anatolybolshakov anatolybolshakov commented Feb 3, 2021 •
edited
Loading

Task name: VSBuildV1

Description:

  • Added input 'enableDefaultLogger' - which allows to disable default logger (it is 'true' by default for backward compatibility).
  • Added 'IsDefaultLoggerEnabled' parameter for related methods - to control default logger usage - updated MSBuildHelpers common package

Documentation changes required: Y

Added unit tests: added tests to cover new input behavior. Amended current tests to consider new parameter.

Attached related issue: #14255

Checklist:

  • Task version was bumped - please check instruction how to do it
  • Checked that applied changes work as expected:
    Tested changes:
  • with 'enableDefaultLogger' disabled - logs with msbuild args:

image

  • with 'enableDefaultLogger' enabled: - logs with msbuild args:

image

@anatolybolshakov anatolybolshakov requested a review from a team February 3, 2021 19:54
DaniilShmelev
DaniilShmelev approved these changes Feb 4, 2021
View reviewed changes
Copy link
Contributor

@DaniilShmelev DaniilShmelev left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but take a loot at the suggestion and the CI issue

Tasks/Common/MSBuildHelpers/InvokeFunctions.ps1 Outdated Show resolved Hide resolved
@anatolybolshakov anatolybolshakov requested a review from a team February 5, 2021 13:54
@anatolybolshakov anatolybolshakov force-pushed the users/anatolybolshakov/fix14255_vsbuild_default_logger branch from de7f5ae to deddedb Compare February 10, 2021 18:07
@anatolybolshakov anatolybolshakov force-pushed the users/anatolybolshakov/fix14255_vsbuild_default_logger branch from 12e5968 to cbcf1eb Compare February 11, 2021 11:16
@anatolybolshakov anatolybolshakov force-pushed the users/anatolybolshakov/fix14255_vsbuild_default_logger branch from 92752b9 to 75d9f5f Compare February 11, 2021 17:48
alexander-smolyakov
alexander-smolyakov approved these changes Feb 12, 2021
View reviewed changes
Copy link
Contributor

@alexander-smolyakov alexander-smolyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@anatolybolshakov anatolybolshakov merged commit f920a64 into master Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaniilShmelev DaniilShmelev DaniilShmelev approved these changes

@alexander-smolyakov alexander-smolyakov alexander-smolyakov approved these changes

@jtpetty jtpetty Awaiting requested review from jtpetty

@leantk leantk Awaiting requested review from leantk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@anatolybolshakov @alexander-smolyakov @DaniilShmelev