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

feat: add tag resources #32

Merged
merged 1 commit into from
Feb 4, 2022
Merged

Conversation

Fuochi
Copy link
Contributor

@Fuochi Fuochi commented Feb 4, 2022

  • add GetTag
  • add DeleteTag
  • add Tag methods for Prowlarr (not very useful)
  • edit UpdateTag to be aligned with api docs

Copy link
Contributor

@davidnewhall davidnewhall left a comment

Choose a reason for hiding this comment

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

This is beautiful. Thanks!


// UpdateTagContext updates the label for a tag.
func (p *Prowlarr) UpdateTagContext(ctx context.Context, tagID int, label string) (int, error) {
body, err := json.Marshal(&starr.Tag{Label: label, ID: tagID})
Copy link
Contributor

Choose a reason for hiding this comment

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

dope.

}

// UpdateTag updates the label for a tag.
func (p *Prowlarr) UpdateTag(tagID int, label string) (int, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Prowlarr the only one with update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, the others already had update, I just changed it to add the ID. It works even without it, but in all api docs but sonarr's the ID is in the definition (e.g. https://radarr.video/docs/api/#/Tag/put-tag-id)

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, cool. I think something I wrote uses update tag, and it's been around a while so I never made it back there to the rest of the methods.

@davidnewhall davidnewhall merged commit 47a4dfa into golift:master Feb 4, 2022
@davidnewhall
Copy link
Contributor

I'll cut a new release in the next couple weeks. Got a few other changes that are pending testing. Feel free to add anything else you need.

@Fuochi
Copy link
Contributor Author

Fuochi commented Feb 4, 2022

Thanks! I really like this work, I'll try few things with the tags. Once I finished, I'll probably contribute more! But it could take me a while.

btw, do you have in plan to write any unit tests?

@Fuochi Fuochi deleted the feature/tag branch February 4, 2022 10:01
@davidnewhall
Copy link
Contributor

I'd love tests, I suppose I could start a few and get a base going at least. I'm just lazy on this one. :)
I did add some to the new submodule I wrote in another PR... so I'm trying!

Thanks again!

@davidnewhall
Copy link
Contributor

fwiw, I started some tests. If you have feedback on where I'm going with those, I'm all ears.

#33

@Fuochi Fuochi mentioned this pull request Feb 17, 2022
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.

2 participants