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

refactor: implement common check methods by check base #135

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

lvlcn-t
Copy link
Member

@lvlcn-t lvlcn-t commented May 20, 2024

⚠ Review after #134 has been merged, since this PR is based on that one.

Motivation

Currently, we have a lot of duplicated code for each check. This PR aims to refactor the code to have a common base struct which implements the methods that are common for all checks.

Partly addresses #99 but could be extended to generate the wrapping methods and method shells/types for each check.

Changes

I've removed the duplicated code and adjusted our common base struct to implement the methods that are common for all checks. This way we can reduce the amount of code and make it easier to maintain.

To do so I've made it a generic struct that takes in the config type of an individual check. This way we can embed the common check base struct in each check and pass the config to the common check base struct.

For additional information look at the commits.

Tests done

I've moved the tests for the common methods to test the common check base struct. All tests succeeded.

  • Unit tests succeeded
  • E2E tests succeeded

TODO

  • I've assigned this PR to myself
  • I've labeled this PR correctly

@lvlcn-t lvlcn-t added refactoring Refactoring of existing code area/checks Issues/PRs related to Checks labels May 20, 2024
@lvlcn-t lvlcn-t self-assigned this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/checks Issues/PRs related to Checks refactoring Refactoring of existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant