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 #30, Resolve doxygen warning #31

Merged

Conversation

lbleier-GSFC
Copy link
Contributor

@lbleier-GSFC lbleier-GSFC commented Apr 10, 2020

Describe the contribution
Fix #30
Resolve doxygen warnings

Testing performed
Steps taken to test the contribution:

  1. Corrected line(s) that generated warnings
  2. Rebuilt documentation with make doc
  3. Observed no warnings generated
  4. Viewed relevant page(s) to verify correctness

Expected behavior changes
Changes to documentation only; no code impact

Contributor Info - All information REQUIRED for consideration of pull request
Leor Bleier, NASA\GSFC

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.

Actually that path is wrong... might be a remnant of the classic build.?

Correct Path is fsw/platform_inc/to_lab_sub_table.h, relative to wherever the to_lab source resides. In other places I think some documentation uses the cmake-like syntax to indicate the source dir, i.e. ${TO_LAB_SOURCE_DIR}.

Futhermore, this is a header file that really shouldn't be instantiating a table. Header files aren't supposed to instantiate data, just define it. (isn't this a standards issue?) That should probably be a separate ticket though.

Corrected -- I originally missed the leading fsw/

@skliper
Copy link
Contributor

skliper commented Apr 10, 2020

Header files aren't supposed to instantiate data, just define it. (isn't this a standards issue?)

Yes, standards issue and a new ticket, see #32. Just fix the path in this one.

@skliper skliper added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Apr 27, 2020
@skliper
Copy link
Contributor

skliper commented Apr 27, 2020

@lbleier-GSFC can you fix per @jphickey comment?

@lbleier-GSFC
Copy link
Contributor Author

Done

@astrogeco astrogeco added the docs label Apr 27, 2020
@skliper skliper added this to the 2.4.0 milestone Apr 28, 2020
@astrogeco
Copy link
Contributor

@lbleier-GSFC can you update the issue name as "Fix #NUMBER," need the "#" for github to link the issue and we've been using commas as a separator.

@lbleier-GSFC lbleier-GSFC changed the title Fix 30 - fixes for doxygen warning Fix #30 - fixes for doxygen warning Apr 28, 2020
@lbleier-GSFC lbleier-GSFC changed the title Fix #30 - fixes for doxygen warning Fix #30, fixes for doxygen warning Apr 28, 2020
@lbleier-GSFC
Copy link
Contributor Author

@astrogeco Is that better?

@skliper
Copy link
Contributor

skliper commented Apr 28, 2020

autolink still missing, and preferred commit message: "Fix #30, Resolve doxygen warning"

@skliper skliper changed the title Fix #30, fixes for doxygen warning Fix #30, Resolve doxygen warning Apr 28, 2020
@skliper skliper linked an issue Apr 28, 2020 that may be closed by this pull request
@skliper
Copy link
Contributor

skliper commented Apr 28, 2020

Fixed the issue link and updated title, not sure why it wouldn't autolink... did it by hand.

@astrogeco
Copy link
Contributor

CCB 2020-04-29 : APPROVED

@astrogeco astrogeco added CCB-20200429 CCB:Approved Indicates approval by CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels May 5, 2020
@astrogeco astrogeco merged commit e3c530c into nasa:integration-candidate May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates approval by CCB docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix doxygen warnings
4 participants