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

Harmonise stratigraphy DD-179 #19

Merged
merged 2 commits into from
Mar 7, 2023
Merged

Harmonise stratigraphy DD-179 #19

merged 2 commits into from
Mar 7, 2023

Conversation

samleeflang
Copy link
Contributor

@sonarcloud
Copy link

sonarcloud bot commented Mar 6, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

98.2% 98.2% Coverage
26.9% 26.9% Duplication

@samleeflang
Copy link
Contributor Author

samleeflang commented Mar 6, 2023

I know about the duplication and I try to kept it to a minimum (by for example creating the abstractChronostratigraphy class for shared methods). However, I would like to keep a separate class for each term so we can easily change the local if we need more specific logic for that specific term. Also when additional data models come in we can house specific logic in the term class. This means that we will have Terms which only call super methods with specific parameters, they look-a-like a lot but there is no logic getting duplicated (or there shouldn't be)

Copy link
Contributor

@southeo southeo 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. I see what you mean about the duplication. If you want to keep it separate you've done the best you can to minimize it.

@samleeflang samleeflang merged commit 7549f6a into main Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants