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

Response models should be readonly #650

Closed
haacked opened this issue Dec 31, 2014 · 1 comment · Fixed by #658
Closed

Response models should be readonly #650

haacked opened this issue Dec 31, 2014 · 1 comment · Fixed by #658

Comments

@haacked
Copy link
Contributor

haacked commented Dec 31, 2014

It's bothered me that our response models are mutable. They really shouldn't be. Now that I've dug into the serialization code a bit, I think we can make our models have readonly properties. Right now, I'm planning to make the properties have a protected setter instead of private to provide flexibility for others who might be deriving from these classes or testing them.

Let me know what you think. I'll have a PR ready soon-ish. This depends on the work I did in #649

@shiftkey
Copy link
Member

haacked added a commit that referenced this issue Jan 2, 2015
It's bothered me that our response models are mutable. They really
shouldn't be. I made the properties have a protected setter instead of
private to provide flexibility for others who might be deriving from
these classes or testing them.

Fixes #650
haacked added a commit that referenced this issue Jan 2, 2015
khellang added a commit to khellang/octokit.net that referenced this issue Jan 2, 2015
haacked added a commit that referenced this issue Jan 4, 2015
It's bothered me that our response models are mutable. They really
shouldn't be. I made the properties have a protected setter instead of
private to provide flexibility for others who might be deriving from
these classes or testing them.

Fixes #650
haacked added a commit that referenced this issue Jan 4, 2015
khellang added a commit to khellang/octokit.net that referenced this issue Jan 4, 2015
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 a pull request may close this issue.

2 participants