Make `GetLabels()` common to both entity types (pull request and issue)

[shay2025]

Is your feature request related to a problem? Please describe. We have three files:

1. https://github.com/reviewpad/reviewpad/blob/main/codehost/github/target/common_target.go
2. https://github.com/reviewpad/reviewpad/blob/main/codehost/github/target/issue_target.go
3. https://github.com/reviewpad/reviewpad/blob/main/codehost/github/target/pull_request_target.go

The files 2 and 3 deal in different ways towards the same entity parameters according to the entity being deal with.

The file 1 deals in the same way towards the same entity parameters for both the entity types.

Currently, we have the method GetLabels() being implemented in both file 2 and 3.

However, the implementation is exactly the same for both.

Describe the solution you'd like Move the implementation to https://github.com/reviewpad/reviewpad/blob/main/codehost/github/target/common_target.go where it resides the common methods implementation for both entities.

Describe alternatives you've considered None

Additional context None

[michaelc0n]

I can take this one, any additional context or similar pattern to follow? 👍🏽

[shay2025]

Hi @michaelc0n Thanks for your help 🙏🏼I believe there is no more context needed. It is only needed to move the current implementation of GetLabels() that resides in codehost/github/target/issue_target.go and codehost/github/target/pull_request_target.go to codehost/github/target/common_target.go 🙂 Any doubts feel free to ask them!

[michaelc0n]

👋 @shay2025 @marcelosousa ,

added some code here: https://github.com/michaelc0n/reviewpad/compare/main...michaelc0n:reviewpad:Issue339_Make_GetLabels_common_to_both_entity_types?expand=1

Would you please review and let me know your thoughts (whether this is right approach or if you can recommend another? 😄 ), I was able to move things to common however running into 276 failures when running task test

• issue primarily appears to be with tests/mocks, *_target.go code loads without error. any help will be appreciated 👍🏽 , thank you!

[shay2025]

Hi @michaelc0n! We are sorry for the very delayed answer. When running the task test command you should now get no failures if everything is alright, please try again. Also, please open a pull request with your propositions and we will gladly review them 😄

[ferreiratiago]

Hi @michaelc0n :) Can you please give some feedback on this issue? 🙏

[shay2025]

@ferreiratiago I believe this issue can be closed because after some investigation I saw that the method GetLabels is separated for issues and pull request targets because we are getting the labels from the GitHub issues and pull request objects which depend on the target being handled whereas that type of specification does not occur on the methods in the common target interface.