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

Issue.ToUpdate forgets Labels #762

Closed
nigel-sampson opened this issue Mar 19, 2015 · 4 comments
Closed

Issue.ToUpdate forgets Labels #762

nigel-sampson opened this issue Mar 19, 2015 · 4 comments

Comments

@nigel-sampson
Copy link
Contributor

Unsure whether this is a bug or intended but Issue.ToUpdate doesn't copy over the Labels to IssueUpdate. There is a test that asserts the default value to be null which I assume means no change. However that property then behaves differently to the other properties of IssueUpdate.

I came across this when I wanted to remove a Label, this then entrails copying over the Labels and then removing the one I don't want.

Can submit a PR if wanted.

@shiftkey
Copy link
Member

Heh, was waiting to see when I'd be back in this code. Removing one label isn't a scenario I'd thought about when I was in #718.

Perhaps a method AddLabels that takes an enumerable of labels? Would that make you happier?

cc @thedillonb

@nigel-sampson
Copy link
Contributor Author

Given my code the new overload wouldn't matter but I imagine it could be helpful to other people. It's enough for me to know this intended behaviour.

Am curious why we treat Labels different and leave as (null / no change) but don't do the same for the others such as Body? (unless that behaviour has changed as well).

@shiftkey
Copy link
Member

Am curious why we treat Labels different and leave as (null / no change) but don't do the same for the others such as Body?

The milestone and labels are handled slightly differently to the other fields in the API on PATCH: https://developer.github.com/v3/issues/#edit-an-issue

I know I can make this clearer in our API, this feedback is great ✨

@nigel-sampson
Copy link
Contributor Author

Yeah I just discovered the IssueUpdate.Milestone null vs new int?().

Thanks

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

No branches or pull requests

2 participants