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

Refactored validators into separate files #879

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

Onager
Copy link
Contributor

@Onager Onager commented Jun 5, 2024

For ease of viewing and isolating

@Onager Onager requested a review from tomchop June 5, 2024 12:56
Copy link
Collaborator

@tomchop tomchop left a comment

Choose a reason for hiding this comment

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

a few questions, otherwise LGTM!

# Source: https://cloud.google.com/compute/docs/regions-zones/
# Fetched 2023-01-13
# TODO - Fetch at runtime?
_zones = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we make this a module-level constant now that it's in its own module?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment for other modules that have similar attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, done.



@classmethod
def RegisterValidator(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Other manages have a boolean parameter that allows for overwriting validators (useful if we have different internal validation logic)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an override argument here

@Onager Onager requested a review from tomchop June 7, 2024 08:49
@Onager Onager merged commit cdbcc3b into log2timeline:main Jun 7, 2024
9 checks passed
wajihyassine pushed a commit to wajihyassine/dftimewolf that referenced this pull request Jun 12, 2024
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