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: filter repositories using topics #320

Merged
merged 7 commits into from
Feb 13, 2023
Merged

Conversation

raffis
Copy link
Contributor

@raffis raffis commented Feb 2, 2023

What does this change

feat: filter github repositories using topics

What issue does it fix

We have hundreds of repositories and targeting them all or providing a list of repositories is annoying to do so.
It would be nicer just to target repos filtered by topic name.
For example I'd like to add a fix targeting all repos labeled backend.

Checklist

  • Made sure the PR follows the CONTRIBUTING.md guidelines
  • Tests if something new is added

Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
Copy link
Owner

@lindell lindell left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This is definitely something useful that should be added. However, there are two questions that needs to be answered first.

  1. Are topics supported on other platforms? Should be an easy implementation in that case, and I'm happy to test it out if needed.
  2. Multiple topics to be filtered are supported in the current implementation, which should be the case. But I'm unsure if it should be that all topics specified must exist, or if any of the topics has to exist. (The latter is implemented). Any input here? :)

internal/scm/github/util.go Outdated Show resolved Hide resolved
internal/scm/github/github_test.go Show resolved Hide resolved
@raffis
Copy link
Contributor Author

raffis commented Feb 2, 2023

1. Are topics supported on other platforms? Should be an easy implementation in that case, and I'm happy to test it out if needed.

I did not check yet as I don' use any other platform atm. But might have a look if I find the time.

2. Multiple topics to be filtered are supported in the current implementation, which should be the case. But I'm unsure if it should be that all topics specified must exist, or if any of the topics has to exist. (The latter is implemented). Any input here? :)

I asked myself the same question. I came to the conclusion that it makes more sense to match if any exists. Although this might be a matter of taste. But it should not get complicated for a user to configure it so I went with the later. Might just be a matter of documentation.

@lindell
Copy link
Owner

lindell commented Feb 2, 2023

Taking a quick look, it seems that topic exist in:

✔️ GitHub
✔️ GitLab
❌ BitBucket server
✔️ Gitea

If you are unfamiliar with GitLab and GitLab, I could add it in this PR.

@raffis
Copy link
Contributor Author

raffis commented Feb 2, 2023

Taking a quick look, it seems that topic exist in:

heavy_check_mark GitHub heavy_check_mark GitLab x BitBucket server heavy_check_mark Gitea

I will have a look 👍🏻

Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
@raffis
Copy link
Contributor Author

raffis commented Feb 3, 2023

Added support for gitlab and gita. Although I doubt topics is a widely used feature in gitea since I didn't even see the option in the ui to set topics.
Will add a story test once everything else is confirmed.

Copy link
Owner

@lindell lindell left a comment

Choose a reason for hiding this comment

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

Great with the added SCMS! Looks good, but there are some minor changes to be made.

internal/scm/utils/slice.go Outdated Show resolved Hide resolved
internal/scm/utils/slice.go Outdated Show resolved Hide resolved
internal/scm/gitea/gitea.go Outdated Show resolved Hide resolved
internal/scm/repository.go Outdated Show resolved Hide resolved
Copy link
Owner

@lindell lindell left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Just a test in tests/story_test.go and it should be ready to merge 😄

@lindell
Copy link
Owner

lindell commented Feb 8, 2023

Thanks for the contribution 😄 Are you ready to have it merged?

@lindell lindell changed the title feat: filter github repositories using topics feat: filter repositories using topics Feb 8, 2023
@raffis
Copy link
Contributor Author

raffis commented Feb 13, 2023

Thanks for the contribution smile Are you ready to have it merged?

If we agree how the topics are filtered then yeah lets 🚢

@lindell lindell merged commit d3c5403 into lindell:master Feb 13, 2023
@github-actions
Copy link
Contributor

Included in release v0.44.0 🎉

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