Skip to content

Resolved issues 772, 827, 909; added numerous unit tests; code cleanup#915

Merged
uweseimet merged 170 commits intodevelopfrom
feature_issues_and_tests
Oct 23, 2022
Merged

Resolved issues 772, 827, 909; added numerous unit tests; code cleanup#915
uweseimet merged 170 commits intodevelopfrom
feature_issues_and_tests

Conversation

@uweseimet
Copy link
Copy Markdown
Contributor

@uweseimet uweseimet commented Oct 15, 2022 •
edited
Loading

This PR resolves 3 issues which all required moving existing code between classes. The most important changes:

  • Resolved issues 772, 827, 909
  • Improved error handling, e.g. more precise SCSI error codes
  • Improved encapsulation
  • Removed duplicate code
  • Added numerous unit tests, total test count is 260

This was linked to issues Oct 15, 2022
Support SEND DIAGNOSTIC for all device types #909
Closed
Extract StorageDevice class from Disk class #722
Closed
Support RESERVE/RELEASE for all device types, not only for SCLP #827
Closed
@uweseimet uweseimet mentioned this pull request Oct 15, 2022
Closed
@rdmark
Copy link
Copy Markdown
Member

rdmark commented Oct 17, 2022

@uweseimet FYI, Tony was busy taking care of a family situation this weekend so the release hasn't been tagged yet.

@rdmark
Copy link
Copy Markdown
Member

rdmark commented Oct 21, 2022

@uweseimet The 22.10 release has been tagged now. We can unfreeze the develop branch.

@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Oct 22, 2022 •
edited
Loading

I noticed that there is a 22.10.01 tag now, so I assume the develop branch is open again for merging. I am going to finalize this PR.
Edit: @rdmark Just noticed your comment now. Don't know why, but I did not get an email notification for it.

@uweseimet uweseimet marked this pull request as ready for review October 22, 2022 07:01
@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Oct 22, 2022 •
edited
Loading

@rdmark By the way, would you mind waiting for my upcoming changes for issue 911 to be merged before restructuring? These changes add, rename and remove some files, and it would be great if this could be done before restructuring.
After the changes for issue 911 I have not planned any changes that would collide with moving files around. I intend to create the PR for 911 right after this PR has been merged.

akuker
akuker requested changes Oct 23, 2022
View reviewed changes
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 23, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

74.7% 74.7% Coverage
0.0% 0.0% Duplication

@akuker
Copy link
Copy Markdown
Member

akuker commented Oct 23, 2022

Approved. Feel free to merge this @uweseimet . Thanks!

@uweseimet uweseimet merged commit f3553c5 into develop Oct 23, 2022
@uweseimet uweseimet deleted the feature_issues_and_tests branch October 23, 2022 19:51
@uweseimet
Copy link
Copy Markdown
Contributor Author

uweseimet commented Oct 23, 2022

@akuker Thank you. I just merged and added a summary of the most relevant changes to the merge message.

@rdmark rdmark mentioned this pull request Oct 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akuker akuker akuker approved these changes

@erichelgeson erichelgeson Awaiting requested review from erichelgeson

@rdmark rdmark Awaiting requested review from rdmark rdmark is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@uweseimet @rdmark @akuker