Skip to content

Conversation

@ritheshn
Copy link
Collaborator

@ritheshn ritheshn commented Mar 4, 2024

Files:

  • bootshared.h
  • componentInfo.h
  • configShared.h
  • factory.h
  • mempool.h
  • mempoolAccessor.h

@github-actions github-actions bot added AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) AT: CLANG-FORMAT PASS AT: CMAKE-FORMAT PASS and removed AT: WIP Mark PR as a Work in Progress (No Autotesting Performed) labels Mar 4, 2024
@github-actions
Copy link

github-actions bot commented Mar 4, 2024

CMAKE-FORMAT TEST - PASSED

@github-actions
Copy link

github-actions bot commented Mar 4, 2024

CLANG-FORMAT TEST - PASSED

@sst-autotester
Copy link
Contributor

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

Copy link
Contributor

@feldergast feldergast left a comment

Choose a reason for hiding this comment

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

Some of these changes are missing important nuances. When I get more time, I will go through and make some detailed comments, but that probably won’t happen until at least next week.


/**
* @return True if parent insert statistics are shared with the child
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Should instead be "True if child can insert its statistics into its parent's statistics set"

Copy link
Contributor

@gvoskuilen gvoskuilen left a comment

Choose a reason for hiding this comment

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

Commented on a few things that should be fixed.
These are not comprehensive, still waiting on a review from @feldergast.


/**
* @return True if the environment option is enabled
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

True if environment configuration is disabled


/**
* @return Logger
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Return verbosity level

/**
* @brief hasLibrary Checks to see if library exists and can be loaded
* @param elemlib
* @param elemlib Name of the Element Library Database
Copy link
Contributor

Choose a reason for hiding this comment

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

Just "Name of the element library" (not database)

@sst-autotester
Copy link
Contributor

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants