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

Protect TNReaction constructors in NDI_FOUND to suppress unreachable … #819

Merged
merged 1 commit into from
May 5, 2020

Conversation

keadyk
Copy link
Collaborator

@keadyk keadyk commented May 4, 2020

…code warning in MSVC

Background

  • PR 818 caused a build warning for MSVC in release mode; this attempts to fix it.

Purpose of Pull Request

  • See above -- fixing a build warning in MSVC that I introduced in my last PR

Description of changes

  • Move TNReaction constructors inside NDI_FOUND macro; if !NDI_FOUND, constructors are now stubs, and the base class (NDI_Base) will throw.
  • Remove cdi_ndi/config include in derived TNReaction class; it is available through the base class NDI_Base
  • only declare load_ndi function when NDI_FOUND

Status

@keadyk keadyk requested a review from KineticTheory May 4, 2020 17:53
@codecov
Copy link

codecov bot commented May 4, 2020

Codecov Report

Merging #819 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           develop    #819   +/-   ##
=======================================
  Coverage     94.2%   94.2%           
=======================================
  Files          367     367           
  Lines        17259   17259           
=======================================
  Hits         16263   16263           
  Misses         996     996           

Copy link
Collaborator

@KineticTheory KineticTheory left a comment

Choose a reason for hiding this comment

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

LGTM.

@KineticTheory KineticTheory merged commit 24375ce into lanl:develop May 5, 2020
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.

2 participants