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

Add an explicit "EnergyPlus" namespace to a state argument #8667

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

jasondegraw
Copy link
Member

@jasondegraw jasondegraw commented Mar 29, 2021

Pull request overview

This is a defect, but is so small I didn't bother with the issue

There's one function in CoilCoolingDXCurveFitPerformance.cc that is missing the explicit EnergyPlus namespace on the state argument. On Visual Studio 16.3.8, this triggers errors about an ambiguous symbol:

4>C:\...\EnergyPlus\src\EnergyPlus\Coils\CoilCoolingDXCurveFitPerformance.cc(535,67): error C2872: 'EnergyPlusData': ambiguous symbol
4>C:\...\EnergyPlus\src\EnergyPlus/InputProcessing/InputProcessor.hh(72,8): message : could be 'EnergyPlusData'
4>C:\...\EnergyPlus\src\EnergyPlus/TempSolveRoot.hh(68,8): message : or       'EnergyPlus::EnergyPlusData'

Newer versions (and maybe older versions) of Visual Studio do not have this problem. I'm currently stuck on this version for a while, or else I would just update. I think the using namespace EnergyPlus; at the top of the file is probably the thing that's really to blame, but this change at least makes the argument declaration all the same.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@jasondegraw jasondegraw added the Defect Includes code to repair a defect in EnergyPlus label Mar 29, 2021
@mjwitte mjwitte added the DoNotPublish Includes changes that shouldn't be reported in the changelog label Mar 29, 2021
@mjwitte mjwitte added this to the EnergyPlus 9.5.0 milestone Mar 29, 2021
@mjwitte mjwitte merged commit 1221990 into develop Mar 29, 2021
@mjwitte mjwitte deleted the namespace-add branch March 29, 2021 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Includes code to repair a defect in EnergyPlus DoNotPublish Includes changes that shouldn't be reported in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants