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 that are forks #341

Merged
merged 3 commits into from
Apr 9, 2023
Merged

feat: filter repositories that are forks #341

merged 3 commits into from
Apr 9, 2023

Conversation

martincostello
Copy link
Contributor

@martincostello martincostello commented Apr 7, 2023

What does this change

Add support for filtering out repositories that are forks using --skip-forks.

What issue does it fix

See #340.

Notes for the reviewer

None.

Checklist

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

Add support for filtering out repositories that are forks.
See #340.
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 remove a few lines of dead code.

@@ -47,3 +49,7 @@ func (r repository) DefaultBranch() string {
func (r repository) FullName() string {
return fmt.Sprintf("%s/%s", r.ownerName, r.name)
}

func (r repository) Fork() bool {
Copy link
Owner

Choose a reason for hiding this comment

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

I assume this is a relic from when an implementation where the filtering was supposed to be done in the multigitter package instead? I would have been fine with that (only drawback is that there are potential that SCMs allow the filtering to be done at API level). But now this should be dead code?

Copy link
Owner

Choose a reason for hiding this comment

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

Same is true for the field in the repository struct?

Remove unused function.
Remove unused field.
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 👍

Thanks for the contribution 😄

@lindell lindell merged commit 941731b into lindell:master Apr 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 9, 2023

Included in release v0.46.0 🎉

@martincostello martincostello deleted the add-skip-forks branch April 9, 2023 10:50
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