Skip to content

Conversation

@micha91
Copy link
Collaborator

@micha91 micha91 commented May 16, 2025

No description provided.

@micha91 micha91 marked this pull request as ready for review May 19, 2025 05:22
@micha91 micha91 requested a review from ewuerger May 19, 2025 05:22
Copy link
Collaborator

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

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

Didn't mean to find so many things. After resolving them I would write:

LGTM.

Comment on lines 193 to 203
assert len(items) == 1
assert isinstance(
items[0].scope, dm.TestRecord
), "For parameters of TestRecords the scope must be a TestRecord"
response = delete_test_record_test_parameter.sync_detailed(
self._project_id,
items[0].scope.test_run_id,
items[0].scope.work_item_project_id,
items[0].scope.work_item_id,
str(items[0].scope.iteration),
items[0].name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert len(items) == 1
assert isinstance(
items[0].scope, dm.TestRecord
), "For parameters of TestRecords the scope must be a TestRecord"
response = delete_test_record_test_parameter.sync_detailed(
self._project_id,
items[0].scope.test_run_id,
items[0].scope.work_item_project_id,
items[0].scope.work_item_id,
str(items[0].scope.iteration),
items[0].name,
assert len(items) == 1
test_parameter = items[0]
assert isinstance(
test_parameter.scope, dm.TestRecord
), "For parameters of TestRecords the scope must be a TestRecord"
response = delete_test_record_test_parameter.sync_detailed(
self._project_id,
test_parameter.scope.test_run_id,
test_parameter.scope.work_item_project_id,
test_parameter.scope.work_item_id,
str(test_parameter.scope.iteration),
test_parameter.name,

Better readability.

Copy link
Collaborator

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

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

Last suggestion.

Comment on lines 192 to 193
def _delete(self, items: list[dm.TestParameter]):
assert len(items) == 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def _delete(self, items: list[dm.TestParameter]):
assert len(items) == 1
def _delete(self, items: list[dm.TestParameter]):
"""Delete a single test parameter."""
assert len(items) == 1

@micha91 micha91 requested a review from ewuerger May 23, 2025 15:51
Copy link
Collaborator

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

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

LGTM.

@micha91 micha91 merged commit b43ba44 into main May 26, 2025
8 checks passed
@ewuerger ewuerger deleted the test-parameters branch June 17, 2025 11:38
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.

3 participants