Skip to content

Conversation

@Evangelink
Copy link
Member

Description

Remove enabled check when the log doesn't involve heavy load of parameters as the method call already checks for it.

@MarcoRossignoli
Copy link
Contributor

@Evangelink there're some issue with tests

{
EqtTrace.Info("DataCollectionTestCaseEventHandler: Test case '{0} - {1}' started.", testCaseStartEventArgs.TestCaseName, testCaseStartEventArgs.TestCaseId);
}
EqtTrace.Info("DataCollectionTestCaseEventHandler: Test case '{0} - {1}' started.", testCaseStartEventArgs?.TestCaseName, testCaseStartEventArgs?.TestCaseId);
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to have a different log in case of a null testCaseStartEventArgs?

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

EqtTrace.Info("DataCollectionTestCaseEventHandler: Test case '{0} - {1}' completed", testCaseEndEventArgs.TestCaseName, testCaseEndEventArgs.TestCaseId);
}

EqtTrace.Info("DataCollectionTestCaseEventHandler: Test case '{0} - {1}' completed", testCaseEndEventArgs?.TestCaseName, testCaseEndEventArgs?.TestCaseId);
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to have a different log in case of a null testCaseEndEventArgs?

Copy link
Contributor

@MarcoRossignoli MarcoRossignoli Feb 11, 2022

Choose a reason for hiding this comment

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

I think it's a guard for something not fundamental...we don't expect a null at all and maybe in that case we'll fail somewhere before

@Evangelink Evangelink merged commit 8814a6d into microsoft:main Feb 11, 2022
@Evangelink Evangelink deleted the simpliy-eqttrace-calls branch February 11, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants