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

WIP: Representation items table #397

Merged
merged 6 commits into from
Apr 13, 2021

Conversation

Elvira2227
Copy link
Collaborator

@Elvira2227 Elvira2227 commented Mar 11, 2021

Entitys was made until IfcGeometricRepresentationItem ->IfcSurface

Fixes #389 .

@Elvira2227 Elvira2227 added documentation Improvements or additions to documentation example-files Issue related to failing reading an example file IFC Content related to Industry Foundation Classes (IFC) functionalities todo_memo Memo on left over TODOs in code labels Mar 11, 2021
@pjanck pjanck changed the title Work in progress WIP: Representation items table Mar 11, 2021
Copy link
Contributor

@pjanck pjanck left a comment

Choose a reason for hiding this comment

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

Some comments from the top of my head.

Also: we have plenty of unit tests for some of these already rolling. Make sure to check functions called "CheckForEssentialEntity" or sth like that in the unit tests.

NOTE for comments where there is no message "Comment on lines x-y" - these only apply to the last line shown in the code window (github always shows the line + three predecessors).

Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
@pjanck pjanck marked this pull request as draft March 11, 2021 15:47
@pjanck pjanck removed the todo_memo Memo on left over TODOs in code label Mar 11, 2021
@Elvira2227 Elvira2227 requested a review from pjanck March 14, 2021 17:27
Copy link
Contributor

@pjanck pjanck left a comment

Choose a reason for hiding this comment

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

Great work so far!

When you think that you've included all issues (some are still missing, I believe), mark the pull request as "ready for review".

Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
@Elvira2227 Elvira2227 requested a review from pjanck April 8, 2021 16:18
@pjanck pjanck marked this pull request as ready for review April 9, 2021 06:02
Copy link
Contributor

@pjanck pjanck left a comment

Choose a reason for hiding this comment

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

Small corrections needed. Otherwise LFTM.

Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Documentation/markdown/SupportedIFCrepresentations.md Outdated Show resolved Hide resolved
Copy link
Contributor

@pjanck pjanck left a comment

Choose a reason for hiding this comment

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

One more comment unresolved, otherwise this is finished.

@Elvira2227 Elvira2227 requested a review from pjanck April 12, 2021 19:05
@pjanck pjanck merged commit aeb0a60 into tumcms:development Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation example-files Issue related to failing reading an example file IFC Content related to Industry Foundation Classes (IFC) functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TODO] Fill in support of IFC representations
3 participants