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

Swig csharp fixes #3959

Merged
merged 13 commits into from
Jun 2, 2020
Merged

Swig csharp fixes #3959

merged 13 commits into from
Jun 2, 2020

Conversation

jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Apr 27, 2020 •
edited
Loading

Pull request overview

  • Fixes Can't Add a MeasureType in C# Bindings #2795
  • Generally try to improve the C# bindings by doing two things:
    • Avoid the LOTS of warnings we are getting (eg: overload resolution)
    • Reduce the number of improperly swig'ed types (SWIGTYPE_p) outside of model (which got most of the attention until now)

Pull Request Author

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

  • Model API Changes / Additions
  • Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
  • Model API methods are tested (in src/model/test)
  • EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
  • If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
  • If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
  • Checked behavior in OpenStudioApplication, adjusted policies as needed (src/openstudio_lib/library/OpenStudioPolicy.xml)
  • Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
  • All new and existing tests passes
  • If methods have been deprecated, update rest of code to use the new methods

Labels:

  • If change to an IDD file, add the label IDDChange
  • If breaking existing API, add the label APIChange
  • If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • Code Style, strip trailing whitespace, etc.
  • All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
  • Labeling is ok
  • 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

tijcolem and others added 11 commits April 20, 2020 12:29
@tijcolem
add check for Ruby Installer before calling it
6887a0f
@jmarrec
Merge remote-tracking branch 'origin/develop' into v3.0.0-rc3
5837217
@jmarrec
whitespace changes in bundle tests [chore]
850f74a
@jmarrec
Fix #3311: use workflow gem instead of standards to skin down disk usage
d7e0bec
@jmarrec
Avoid some C# annoying warnings about 'xxx' is a C# keyword
ba10eff
@jmarrec
Avoid thousands of echo "cwstring.i not implemented for this target"
d6f24cc 
cwstring.i is only implemented for python and tcl really.
@jmarrec
Help in overload resolution to suppress C# warning 516
be5c801
@jmarrec
Resolve some SWIGTYPE_p problems
a4ab9be
@jmarrec
Try to avoid warning overloaded toString, not exactly working still.
126aa7d
@jmarrec
SWIG std::pair<VersionString, std::string>. But VersionString needs a…
b3d9f7c 
… default constructor, so make one
@jmarrec
Add a proper helper class for ScheduleTypeKey
dca3876 
Swig isn't happy if you do two typedef/templates for the same stuff (std::pair<std::string, std::string> here)
@jmarrec jmarrec added component - C# Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Apr 27, 2020
@jmarrec jmarrec self-assigned this Apr 27, 2020
jmarrec added 2 commits April 27, 2020 20:26
@jmarrec
Warnign treated as error on MSVC
bf1ba58
@jmarrec
Ignore error CS0108 which throws a ton of warning due to iddObjectTyp…
1b58ace 
…e hiding inherited

````
Severity	Code	Description	Project	File	Line	Suppression State
Warning	CS0108	'MasslessOpaqueMaterial.iddObjectType()' hides inherited member 'ModelObject.iddObjectType()'. Use the new keyword if hiding was intended. [D:\OpenStudio\build-vs\csharp_wrapper\OpenStudio.csproj]	csharp_sdk	D:\OpenStudio\build-vs\csharp\generated_sources\OpenStudioModelResources\MasslessOpaqueMaterial.cs	81	
```
@jmarrec jmarrec mentioned this pull request Apr 28, 2020
Merged
24 tasks
@jmarrec jmarrec requested a review from kbenne April 28, 2020 13:57
@jmarrec
Copy link
Collaborator Author

jmarrec commented Apr 28, 2020

@kbenne Ok this is ready for review I think. It builds fine on Windows and the build output is MUCH cleaner: https://ci.commercialbuildings.dev/blue/rest/organizations/jenkins/pipelines/openstudio-incremental-develop/branches/PR-3959/runs/4/nodes/7/steps/161/log/?start=0

jmarrec added a commit that referenced this pull request Jun 2, 2020
@jmarrec
Add release notes for #3959 and #3960 for 3.0.1
d6a6525
@kbenne kbenne merged commit 1b58ace into develop Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbenne kbenne Awaiting requested review from kbenne

Assignees

@jmarrec jmarrec

Labels
component - C# Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't Add a MeasureType in C# Bindings
3 participants
@jmarrec @kbenne @tijcolem