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

LifeCycleAssessment_oM: Update EPD Object Properties #1004

Merged
merged 7 commits into from
Oct 12, 2020
Merged

LifeCycleAssessment_oM: Update EPD Object Properties #1004

merged 7 commits into from
Oct 12, 2020

Conversation

michaelhoehn
Copy link
Contributor

@michaelhoehn michaelhoehn commented Oct 2, 2020 •
edited
Loading

NOTE: Depends on

Issues addressed by this PR

Closes #1003
Closes #941
Closes #1007
Closes #793
Closes #791

Additional object properties for more robust data where applicable.
Default density value set to 0 instead of double.NaN for increased engine method reliability.

Test files

Test File

Changelog

Additional comments

@michaelhoehn michaelhoehn added the type:feature New capability or enhancement label Oct 2, 2020
@michaelhoehn michaelhoehn added this to the BHoM 4.0 β MVP milestone Oct 2, 2020
This was referenced Oct 2, 2020
Merged
Closed
@michaelhoehn
Copy link
Contributor Author

/azp run BHoM.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@michaelhoehn michaelhoehn self-assigned this Oct 6, 2020
FraserGreenroyd
FraserGreenroyd requested changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Minor description updates - also, can we use any Quantity attributes on any of the properties?

LifeCycleAssessment_oM/MaterialFragment/EnvironmentalProductDeclaration.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/EnvironmentalProductDeclaration.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/IEnvironmentalProductDeclarationData.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/SectorEnvironmentalProductDeclaration.cs Outdated Show resolved Hide resolved
@BHoM BHoM deleted a comment from azure-pipelines bot Oct 6, 2020
@michaelhoehn
Copy link
Contributor Author

/azp run BHoM.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@michaelhoehn
Copy link
Contributor Author

/azp run BHoM.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

FraserGreenroyd
FraserGreenroyd requested changes Oct 8, 2020
View reviewed changes
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Minor things.

Also, can we put the Quantity attributes as the first attribute, which matches what we've done on other objects in Environment/MEP/Structure oM 😄

LifeCycleAssessment_oM/MaterialFragment/EnvironmentalProductDeclaration.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/EnvironmentalProductDeclaration.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/IEnvironmentalProductDeclarationData.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/IEnvironmentalProductDeclarationData.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/SectorEnvironmentalProductDeclaration.cs Outdated Show resolved Hide resolved
LifeCycleAssessment_oM/MaterialFragment/SectorEnvironmentalProductDeclaration.cs Outdated Show resolved Hide resolved
ajensen19
ajensen19 previously approved these changes Oct 8, 2020
View reviewed changes
Copy link

@ajensen19 ajensen19 left a comment

Choose a reason for hiding this comment

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

yay! i finally figured out how to complete a review ;) looks good!

@michaelhoehn
Requested changes
1a8b27c 
Attribute syntax adjustment + Plant Description update
@michaelhoehn
Copy link
Contributor Author

yay! i finally figured out how to complete a review ;) looks good!

Exciting @ajensen19! Now do it again to cover the changes requested by @FraserGreenroyd 😛

@BHoM BHoM deleted a comment from azure-pipelines bot Oct 9, 2020
@michaelhoehn
Edit Properties
d5f2949 
Remove Scope + Add QuantityTypeValue
@michaelhoehn
Copy link
Contributor Author

/azp run BHoM.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BHoM BHoM deleted a comment from azure-pipelines bot Oct 9, 2020
@BHoM BHoM deleted a comment from azure-pipelines bot Oct 9, 2020
FraserGreenroyd
FraserGreenroyd approved these changes Oct 9, 2020
View reviewed changes
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks @michaelhoehn

FraserGreenroyd
FraserGreenroyd requested changes Oct 9, 2020
View reviewed changes
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Blocking till dependency PRs are ready

@michaelhoehn michaelhoehn added the status:do-not-merge For instance, test PR, for discussion, or dependant PRs not ready for merge label Oct 9, 2020
@michaelhoehn michaelhoehn mentioned this pull request Oct 12, 2020
Merged
@michaelhoehn michaelhoehn removed the status:do-not-merge For instance, test PR, for discussion, or dependant PRs not ready for merge label Oct 12, 2020
FraserGreenroyd
FraserGreenroyd approved these changes Oct 12, 2020
View reviewed changes
Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

All dependent PRs are done and ready so this LGTM, thanks @michaelhoehn 😄

@FraserGreenroyd FraserGreenroyd merged commit 669fff7 into master Oct 12, 2020
@FraserGreenroyd FraserGreenroyd deleted the LifeCycleAssessment_oM-#1003-EPD-Property-Updates branch October 12, 2020 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FraserGreenroyd FraserGreenroyd FraserGreenroyd approved these changes

@kayleighhoude kayleighhoude Awaiting requested review from kayleighhoude

@ajensen19 ajensen19 Awaiting requested review from ajensen19

Assignees

@michaelhoehn michaelhoehn

Labels
type:feature New capability or enhancement
Projects
None yet
Milestone
BHoM 4.0 β RC
4 participants
@michaelhoehn @FraserGreenroyd @ajensen19 @kayleighhoude