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

2D Viewer module #811

Merged
merged 10 commits into from
Dec 16, 2024
Merged

2D Viewer module #811

merged 10 commits into from
Dec 16, 2024

Conversation

Co-authored-by: Hans Kallekleiv <[email protected]>
@rubenthoms rubenthoms added the enhancement New feature or request label Nov 18, 2024
@rubenthoms rubenthoms changed the title Implemented new 2d viewer module 2D Viewer module Nov 25, 2024
Copy link
Collaborator

@jorgenherje jorgenherje left a comment

Choose a reason for hiding this comment

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

Really nice job! A lot of great stuff.

Have reviewed everything except 2DViewer/layers/ folder. Will take than after the weekend, so this is a first step of my review 👍

Only minor comments from my side.

@jorgenherje jorgenherje self-requested a review December 6, 2024 14:04
Copy link
Collaborator

@jorgenherje jorgenherje left a comment

Choose a reason for hiding this comment

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

Really nice and impressive work!👍

I've reviewed the general structure and general functionality, without diving too much into details. Just minor comments (sorry for a lot of repeating comments) with respect to naming and documentation for clarity.

In general, I prefer naming classes which implements an interface a name ending with the interface name. E.g. class RealizationItem implements Item {} vs class Realization implements Item {} - when we have an interface named Item. This can make it easier for the user when where RealizationItem component/class is used. As for the classes in the implementations/-folder.

Copy link
Collaborator

@jorgenherje jorgenherje left a comment

Choose a reason for hiding this comment

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

Looks good to me! Great work! 👍

@rubenthoms rubenthoms merged commit bd137c6 into equinor:main Dec 16, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2D viewer v1.0 remaining issues
3 participants