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

Fix #27, Add library support to application checksum computing function. #37

Merged
merged 1 commit into from
Jul 28, 2022
Merged

Fix #27, Add library support to application checksum computing function. #37

merged 1 commit into from
Jul 28, 2022

Conversation

dzbaker
Copy link
Contributor

@dzbaker dzbaker commented Jul 21, 2022 •
edited
Loading

Checklist (Please check before submitting)

Describe the contribution
Fixes #27

Testing performed
CI

Expected behavior changes
CS_ComputeApp() will now scrub libraries in addition to apps when computing the checksum.

Unit test updates (all in cs_compute_tests.c):

  • Separate nominal tests for checking an app vs. library
  • Modified App ID name error test to use invalid App and Lib ID
  • GetModuleInfo() error test case (instead of GetAppInfo())

System(s) tested on
CI

@dzbaker dzbaker force-pushed the fix27-library_scrubbing_support branch from d9ad672 to bdb3d67 Compare July 21, 2022 17:35
skliper
skliper requested changes Jul 21, 2022
View reviewed changes
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Great work! See comments.

unit-test/cs_compute_tests.c Outdated Show resolved Hide resolved
unit-test/cs_compute_tests.c Outdated Show resolved Hide resolved
unit-test/cs_compute_tests.c Outdated Show resolved Hide resolved
unit-test/cs_compute_tests.c Show resolved Hide resolved
@skliper skliper requested a review from dmknutsen July 21, 2022 19:20
dmknutsen
dmknutsen approved these changes Jul 21, 2022
View reviewed changes
Copy link
Contributor

@dmknutsen dmknutsen left a comment

Choose a reason for hiding this comment

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

No comments in addition to what Jake mentioned. Great work!

@dzbaker dzbaker force-pushed the fix27-library_scrubbing_support branch 15 times, most recently from c32a039 to 7b106dc Compare July 26, 2022 15:16
@dzbaker dzbaker requested a review from skliper July 26, 2022 15:23
skliper
skliper requested changes Jul 26, 2022
View reviewed changes
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Just one minor suggestion, good work!

fsw/src/cs_compute.c Outdated Show resolved Hide resolved
@dzbaker dzbaker force-pushed the fix27-library_scrubbing_support branch from 7b106dc to ae3184f Compare July 26, 2022 18:18
@dzbaker dzbaker requested a review from skliper July 26, 2022 18:23
@dzbaker dzbaker merged commit 4280e14 into nasa:main Jul 28, 2022
@skliper skliper added this to the Draco milestone Aug 31, 2022
skliper pushed a commit to skliper/CS that referenced this pull request Sep 9, 2022
@dzbaker
Merge pull request nasa#37 from skliper/fix21-coverage_test
e484dc0 
Fix nasa#15 nasa#21 nasa#36, Code coverage improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper approved these changes

@dmknutsen dmknutsen dmknutsen approved these changes

Assignees
No one assigned
Labels
CCB:Approved draco-rc3
Projects
None yet
Milestone
Draco
Development

Successfully merging this pull request may close these issues.

Add scrubbing support for libraries
3 participants
@dzbaker @skliper @dmknutsen