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

Refactoring 2022 03 31 #1302

Draft
wants to merge 4 commits into
base: skosmos-2
Choose a base branch
from
Draft

Refactoring 2022 03 31 #1302

wants to merge 4 commits into from

Conversation

miguelvaara
Copy link
Contributor

Reasons for creating this PR

Property values in the Skosmos or vocabulary configuration, got by a key should not be called directly, therefore the properties need separate interfaces (methods). This makes it easier to debug the code and the structure of the code will be easier to read, understand and maintain.

Link to relevant issue(s), if any

  • Closes #

Description of the changes in this PR

Refactored those parts of the code in the VocabularyConfig.php where properties were called without dedicated methods. Added methods for the properties without an API.

Tests are needed but they will be made after the first review, after the implementation and the quality of the code have been checked.

Tests are needed but they will be made after the first review, after the idea and the quality of the code is checked.

Known problems or uncertainties in this PR

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if not, explain why below)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@sonarcloud
Copy link

sonarcloud bot commented Apr 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

1 participant