Skip to content

Adding Label description property#475

Merged
bitwiseman merged 9 commits into
hub4j:masterfrom
immanuelqrw:label-description
Oct 5, 2019
Merged

Adding Label description property#475
bitwiseman merged 9 commits into
hub4j:masterfrom
immanuelqrw:label-description

Conversation

@immanuelqrw
Copy link
Copy Markdown
Contributor

Label in the GitHub API has an optional field description which had not been implemented.

Issue #474

Label in the GitHub API has an optional field description which had not been implemented.
Copy link
Copy Markdown

@deadmoose deadmoose left a comment

Choose a reason for hiding this comment

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

Ping!

I've got a little tool to bootstrap some configuration on repos & want to be able to have the machine fully define standard labels.

*/
public void setColor(String newColor) throws IOException {
repo.root.retrieve().method("PATCH").with("name", name).with("color", newColor).to(url);
repo.root.retrieve().method("PATCH")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This (and similarly setDescription()) needs to also update the field on this, otherwise the local GHLabel immediately falls out of sync with the upstream one.

It's a pre-existing bug, but it's exacerbated by adding another mutable field, since you quite possibly want to do something like

GHLabel label = repo.getLabel("foo");
label.setColor("ffffff");
label.setDescription("bar");

Ideally, we'd also be able to update multiple fields in a single network call, but I'll live so long as I don't have to do weird stuff to work around it like

repo.getLabel("foo").setColor("ffffff");
repo.getLabel("foo").setDescription("bar");

You also might not need to send all the fields on the PATCH request, but the API docs don't make it clear & I haven't tried.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assuming that it is needed seems like a low risk choice.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@deadmoose
If you are still around what do you think about using the builder/updater style similar to Release/ReleaseBuilder/ReleaseUpdater?
That way the original object remain unchanged, but you can still update and do it all in one network call. This also sidesteps the question of when all fields need to be sent.

I admit it would be a bit more work to do - We'd want to mark existing set* methods a deprecated, but it wouldn't be that bad.

*/
public void setColor(String newColor) throws IOException {
repo.root.retrieve().method("PATCH").with("name", name).with("color", newColor).to(url);
repo.root.retrieve().method("PATCH")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assuming that it is needed seems like a low risk choice.

@bitwiseman bitwiseman merged commit fdf5d3f into hub4j:master Oct 5, 2019
@immanuelqrw immanuelqrw deleted the label-description branch February 8, 2020 17:15
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.

3 participants