singer-io / tap-gitlab

GNU Affero General Public License v3.0
15 stars 19 forks source link

Added support for groups and group milestones. #9

Closed chrisb-c01 closed 6 years ago

chrisb-c01 commented 6 years ago

This PR adds support for Gitlab Groups and Group milestones to the tap. When syncing groups, one can choose between syncing all projects contained in the group, or make a sub-selection of projects.

cmerrick commented 6 years ago

Hi @ChrisCalculus, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

cmerrick commented 6 years ago

You did it @ChrisCalculus!

Thank you for signing the Singer Contribution License Agreement.

chrisb-c01 commented 6 years ago

Hi @KAllan357,

thanks for your feedback. I've provided fixes as you suggested. Please see my new commit.

In detail:

Could you please provide a bit of a description in this pull request. Just a general summary of what's changing / being added would be perfect.

Done.

Could you add some text to the README.md to indicate that we'll also be syncing groups and group milestones?

Done.

If I understand correctly, this change will require a new field in the Stitch UI. It looks like it wants to perform the same was as our current project field - a space separated list of GitLab groups. Is that correct?

TL;DR: Yes.

As groups can 'contain' projects and the current tap allows for a selection of projects, it seemed logical to me that you'd also want to configure a selection of groups. A user might want to sync all projects contained in a group or only sync a sub-set of projects. I've implemented it in such a way that 3 scenarios are possible:

Note: Unfortunately groups don't provide a date field to be tracked as part of the state.

I've added responses to your individual comments.

KAllan357 commented 6 years ago

Thanks for the detailed responses. This looks good now. :ship:

KAllan357 commented 6 years ago

I'm going to merge this now and release it over the next few days. We'll need a little bit of time to add the new UI element.

chrisb-c01 commented 6 years ago

Great, thanks @KAllan357