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

Tidied up attribute 'type'. #1

Merged
merged 2 commits into from Dec 7, 2019
Merged

Tidied up attribute 'type'. #1

merged 2 commits into from Dec 7, 2019

Conversation

@manwar
Copy link
Contributor

manwar commented Dec 2, 2019

Hi @wreis

I received the distribution as part of December assignment for Pull Request Club.

In this PR, I proposed to make the attribute 'type' more strict. As currently user can set the 'type' attribute in the constructor which gets overridden and ignored silently. I proposed to make the attribute "readonly" and stop user setting in the constructor.

Using init_arg => undef, we can ignore any value provided by the constructor. However it still didn't warn the user. I came across very handy MooseX::StrictConstructor that does the job very nicely.

Please let me know if you want to make any changes to my proposals. I am more than happy to change it as per your taste as it is your baby.

Best Regards,
Mohammad S Anwar

@wreis

This comment has been minimized.

Copy link
Owner

wreis commented Dec 6, 2019

Hi @manwar,

Thanks for the PR. Makes sense yeah. Could you please add tests?

@manwar

This comment has been minimized.

Copy link
Contributor Author

manwar commented Dec 6, 2019

@wreis Added unit test without adding new test prereq.
Please review.

@wreis

This comment has been minimized.

Copy link
Owner

wreis commented Dec 7, 2019

Thank you!

@wreis wreis merged commit 27c2bb6 into wreis:master Dec 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.