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.

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

Fix #1107, Remove unreachable branch (superfluous if condition) #1368

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Feb 15, 2023

Checklist

Describe the contribution

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.

Expected behavior changes
No change to behavior.

Contributor Info
Avi Weiss @thnkslprpt

@dmknutsen dmknutsen added this to the Equuleus milestone Sep 6, 2023
@chillfig chillfig added coverage unit-test Tickets related to the OSAL unit testing (functional and/or coverage) enhancement and removed unit-test Tickets related to the OSAL unit testing (functional and/or coverage) labels Sep 9, 2023
@skliper
Copy link
Contributor

skliper commented Sep 21, 2023

Does this change branch coverage results at all? If so I recommend updating:

allowed_ncov_branches: 4

@thnkslprpt thnkslprpt force-pushed the fix-1107-remove-dead-branch branch from 19dc2da to a357ce4 Compare September 22, 2023 00:16
@thnkslprpt thnkslprpt force-pushed the fix-1107-remove-dead-branch branch from a357ce4 to ea2faf7 Compare September 22, 2023 00:51
@thnkslprpt
Copy link
Contributor Author

Does this change branch coverage results at all? If so I recommend updating:

allowed_ncov_branches: 4

Thanks!

I need a checklist or something - that's the second time I forgot to update these parameters for allowed exceptions to coverage.

Anyway I updated it now - it had actually become out-of-date at some point before this PR (was allowing 4 exceptions when the main branch was already down to only 3 missed branches).

I've lowered it now from 4 to 2.

Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

If I remember correctly, this could be a remnant from the days when there was a global volume table in OSAL that could pre-init these items. I think we've fully purged that, and I concur I can't see any path though the code where this condition could ever be false.

@dmknutsen dmknutsen added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) Equuleus-rc2 labels Oct 19, 2023
@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jan 25, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Jan 31, 2024
*Combines:*

cFE equuleus-rc1+dev84
osal equuleus-rc1+dev41

**Includes:**

*cFE*
- nasa/cFE#2505

*osal*
- nasa/osal#1368

Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Avi Weiss <[email protected]>
@dzbaker dzbaker merged commit 10b0255 into nasa:main Jan 31, 2024
20 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request Jan 31, 2024
*Combines:*

cFE equuleus-rc1+dev84
osal equuleus-rc1+dev41

**Includes:**

*cFE*
- nasa/cFE#2505

*osal*
- nasa/osal#1368

Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Avi Weiss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB coverage enhancement Equuleus-rc2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unreachable branch on unknown fstype in OS_FileSys_Initialize
7 participants