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

Support gitlab_mr_destination #215

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dpoluyanov
Copy link

I've working on support gitlab-related workflows for our company and implemented initial support for gitlab_mr_destination,
It is pretty initial work and mostly based on #214 in ground, but still could be reviewed, while I'm working on tests and documentation update for it

Another step toward #153

Closes #214

@dpoluyanov
Copy link
Author

@mikelalcon, could you please take a look and leave some remarks on it?

@mikelalcon
Copy link
Collaborator

Hi! Thanks for the contribution!

I see that some of the classes (like java/com/google/copybara/git/GitLabMrWriteHook.java) are copies of the GH one. Is it possible to extend Gitlab one from the GitHub one instead?

Regarding to #214, we are currently refactoring this code to allow arbitrary flags but that they can be validated. Give us 1-2 weeks :)

@dpoluyanov
Copy link
Author

Yes, most of classes the same as in github package and probably some code could be extensively reused for github and gitlab integrations but for start I’ve intentionally kept it separate in order to keep it clean and not break internal code at google.
If it’s not a point, I can extend or create common classes for both integrations wherever it suitable

@mikelalcon
Copy link
Collaborator

I think we could have some shared code. I can try to do a refactor (including your code) if it SGTY

@dpoluyanov
Copy link
Author

Sounds good, but let me finish pull request =)

@abitrolly
Copy link

So does copybara support GitLab?

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.

3 participants