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 #2410, document ES Perf enums #2411

Merged
merged 2 commits into from
Dec 12, 2023
Merged

Fix #2410, document ES Perf enums #2411

merged 2 commits into from
Dec 12, 2023

Conversation

irowebbn
Copy link
Contributor

Checklist (Please check before submitting)

Describe the contribution
A clear and concise description of what the contribution is.

Expected behavior changes

  • No impact to behavior (doc change)

Contributor Info - All information REQUIRED for consideration of pull request
Isaac Rowe, NASA JSC, Jacobs Technology

@irowebbn
Copy link
Contributor Author

I'm having trouble rebuilding the user's guide, so I may need assistance with that for testing.

@chillfig chillfig added the docs This change only affects documentation. label Aug 30, 2023
@dzbaker
Copy link
Collaborator

dzbaker commented Nov 30, 2023

CCB 30 November 2023: Approved pending merge conflict resolution.

@dzbaker dzbaker added the CCB:Approved Indicates code review and approval by community CCB label Nov 30, 2023
dzbaker added a commit that referenced this pull request Dec 5, 2023
@dzbaker
Merge pull request #2411 from irowebbn:2410-doc-triggermode
f144f39 
Fix 2410: document ES Perf enums
@dzbaker dzbaker mentioned this pull request Dec 5, 2023
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 5, 2023
@dzbaker
IC: Caelum-rc4+dev65
6a330c5 
*Combines:*

to_lab v2.5.0-rc4+dev71
ci_lab v2.5.0-rc4+dev77
cFE v7.0.0-rc4+dev427
PSP v1.6.0-rc4+dev102
osal v6.0.0-rc4+dev243

**Includes:**

*to_lab*
- nasa/to_lab#173

*ci_lab*
- nasa/ci_lab#157
- nasa/ci_lab#159
- nasa/ci_lab#161

*cFE*
- nasa/cFE#2411
- nasa/cFE#2409
- nasa/cFE#2373
- nasa/cFE#2466
- nasa/cFE#2468
- nasa/cFE#2470

*PSP*
- nasa/PSP#421

*osal*
- nasa/osal#1413

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored by: Frank Kühndel <frank-kue@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Dec 5, 2023
Merged
2 tasks
@dzbaker dzbaker added CCB:Provisionally-Approved and removed CCB:Approved Indicates code review and approval by community CCB labels Dec 5, 2023
@dzbaker
Copy link
Collaborator

dzbaker commented Dec 5, 2023

@irowebbn We're getting some workflow failures on building documentation now. Would you be able to resolve those?

@irowebbn irowebbn force-pushed the 2410-doc-triggermode branch 2 times, most recently from 499b8b4 to 34960e0 Compare December 5, 2023 17:56
@irowebbn
Copy link
Contributor Author

irowebbn commented Dec 5, 2023 •
edited
Loading

@dzbaker , I haven't been able to replicate the cfe-userguide failure locally.

@dzbaker dzbaker requested a review from jphickey December 7, 2023 16:07
jphickey added a commit to irowebbn/cFE that referenced this pull request Dec 7, 2023
@jphickey
Update nasa#2411, move perf enum definition
6d4db95
jphickey added a commit to irowebbn/cFE that referenced this pull request Dec 7, 2023
@jphickey
HotFix nasa#2411, move perf enum definition
90541e8 
The API documentation should not refer to internal headers, only
public headers.  The fact that the message payload was based on
this enum meant that the enum definition is in the wrong place.
@jphickey
Copy link
Contributor

jphickey commented Dec 7, 2023

The issue here was a bit more subtle, not just a mismatched ID.

The API documentation only uses the public headers - not the private ones. The "perf" enum was defined in a private header. This really means that the enum was in the wrong place, its actually part of the public API, not a private type.

Moving the perf enum definition solves the problem.

@jphickey jphickey changed the title Fix 2410: document ES Perf enums Fix #2410: document ES Perf enums Dec 7, 2023
@jphickey jphickey changed the title Fix #2410: document ES Perf enums Fix #2410, document ES Perf enums Dec 7, 2023
@jphickey
Copy link
Contributor

jphickey commented Dec 7, 2023

The only test "failing" now was the format checks.... trying to fix that....

@jphickey
HotFix nasa#2411, move perf enum definition
e56d5e0 
The API documentation should not refer to internal headers, only
public headers.  The fact that the message payload was based on
this enum meant that the enum definition is in the wrong place.
@jphickey
Copy link
Contributor

jphickey commented Dec 7, 2023

@dzbaker - should be good to merge now.
I also submitted nasa/cFS#724 ... the failure of that workflow is a nuisance.

@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Provisionally-Approved labels Dec 12, 2023
@dzbaker dzbaker mentioned this pull request Dec 12, 2023
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 12, 2023
@dzbaker
IC: Caelum-rc4+dev66
e16440a 
*Combines:*

to_lab v2.5.0-rc4+dev75
ci_lab v2.5.0-rc4+dev81
sample_app v1.3.0-rc4+dev69
sch_lab v2.5.0-rc4+dev75
cFE v7.0.0-rc4+dev434
osal v6.0.0-rc4+dev247

**Includes:**

*to_lab*
- nasa/to_lab#176

*ci_lab*
- nasa/ci_lab#165

*sample_app*
- nasa/sample_app#220

*sch_lab*
- nasa/sch_lab#156

*cFE*
- nasa/cFE#2472
- nasa/cFE#2411
- nasa/cFE#2474

*osal*
- nasa/osal#1430

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Dec 12, 2023
Merged
2 tasks
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 12, 2023
@dzbaker
IC: Caelum-rc4+dev66
71b1cc0 
*Combines:*

to_lab v2.5.0-rc4+dev75
ci_lab v2.5.0-rc4+dev81
sample_app v1.3.0-rc4+dev69
sch_lab v2.5.0-rc4+dev75
cFE v7.0.0-rc4+dev434
osal v6.0.0-rc4+dev247

**Includes:**

*to_lab*
- nasa/to_lab#176

*ci_lab*
- nasa/ci_lab#165

*sample_app*
- nasa/sample_app#220

*sch_lab*
- nasa/sch_lab#156

*cFE*
- nasa/cFE#2472
- nasa/cFE#2411
- nasa/cFE#2474

*osal*
- nasa/osal#1430

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 12, 2023
@dzbaker
IC: Caelum-rc4+dev66
f46e585 
*Combines:*

to_lab v2.5.0-rc4+dev75
ci_lab v2.5.0-rc4+dev81
sample_app v1.3.0-rc4+dev69
sch_lab v2.5.0-rc4+dev75
cFE v7.0.0-rc4+dev434
osal v6.0.0-rc4+dev247

**Includes:**

*to_lab*
- nasa/to_lab#176

*ci_lab*
- nasa/ci_lab#165

*sample_app*
- nasa/sample_app#220

*sch_lab*
- nasa/sch_lab#156

*cFE*
- nasa/cFE#2472
- nasa/cFE#2411
- nasa/cFE#2474

*osal*
- nasa/osal#1430

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
@dzbaker dzbaker merged commit 7ae394d into nasa:main Dec 12, 2023
22 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 12, 2023
@dzbaker
IC: Caelum-rc4+dev66
9d3f786 
*Combines:*

to_lab v2.5.0-rc4+dev75
ci_lab v2.5.0-rc4+dev81
sample_app v1.3.0-rc4+dev69
sch_lab v2.5.0-rc4+dev75
cFE v7.0.0-rc4+dev434
osal v6.0.0-rc4+dev247

**Includes:**

*to_lab*
- nasa/to_lab#176

*ci_lab*
- nasa/ci_lab#165

*sample_app*
- nasa/sample_app#220

*sch_lab*
- nasa/sch_lab#156

*cFE*
- nasa/cFE#2472
- nasa/cFE#2411
- nasa/cFE#2474

*osal*
- nasa/osal#1430

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Isaac Rowe <irowebbn@users.noreply.github.com>
Co-authored by: Avi Weiss <thnkslprpt@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jphickey jphickey jphickey approved these changes

Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB docs This change only affects documentation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Values for TriggerMode provided to CFE_ES_StartPerfCmd are not documented
4 participants
@irowebbn @dzbaker @jphickey @chillfig