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

Revert "Merge pull request #72 from Icinga/reflectPtr" #79

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Conversation

lippserd
Copy link
Member

@lippserd lippserd commented Oct 21, 2024

This reverts commit f25f8a0, reversing changes made to a2fd4d4.

While the changes relax the necessary checks for passing a valid pointer, they complicate the tests and still allow non-struct pointer types that implement the validator interface to pass. This negates the benefits, making it simpler to validate the pointer argument in a function as previously done.

refs #72

This reverts commit f25f8a0, reversing
changes made to a2fd4d4.

While the changes relax the necessary checks for passing a valid pointer, they
complicate the tests and still allow non-construct pointer types that implement
the validator interface to pass. This negates the benefits, making it simpler
to validate the pointer argument in a function as previously done.
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Oct 21, 2024
@lippserd lippserd requested a review from yhabteab October 21, 2024 08:56
@lippserd
Copy link
Member Author

I think validatorPtr can be changed to the following to only allow struct pointer:

// validatorPtr combines the [Validator] interface with a pointer constraint.
type validatorPtr[T ~struct{}] interface {
	Validator
	*T
}

Nevertheless, the tests would have to be adapted, and the non-nil checks can't be removed anyway.

@yhabteab yhabteab merged commit 38f518b into main Oct 21, 2024
15 checks passed
@yhabteab yhabteab deleted the revert-72 branch October 21, 2024 09:41
@oxzi oxzi added this to the 0.4.0 milestone Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants