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(gitlab): Add gitlab support #612

Closed
wants to merge 1 commit into from
Closed

Conversation

dark0dave
Copy link
Contributor

@dark0dave dark0dave commented Apr 14, 2024

Description

Adding Gitlab support

Motivation and Context

Much like we support github we should also support gitlab

How Has This Been Tested?

Its still a work in progress

Screenshots / Logs (if applicable)

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have formatted the code with rustfmt.
  • I checked the lints with clippy.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@dark0dave dark0dave requested a review from orhun as a code owner April 14, 2024 10:46
Copy link

welcome bot commented Apr 14, 2024

Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

@codecov-commenter
Copy link

codecov-commenter commented Apr 14, 2024

Codecov Report

Attention: Patch coverage is 24.21053% with 144 lines in your changes are missing coverage. Please review.

Project coverage is 38.96%. Comparing base (d53b905) to head (6cb0a54).
Report is 11 commits behind head on main.

Files Patch % Lines
git-cliff-core/src/gitlab.rs 0.00% 90 Missing ⚠️
git-cliff-core/src/changelog.rs 30.00% 28 Missing ⚠️
git-cliff/src/lib.rs 0.00% 14 Missing ⚠️
git-cliff-core/src/release.rs 81.09% 7 Missing ⚠️
git-cliff/src/logger.rs 0.00% 4 Missing ⚠️
git-cliff-core/src/commit.rs 50.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #612      +/-   ##
==========================================
- Coverage   41.52%   38.96%   -2.56%     
==========================================
  Files          15       16       +1     
  Lines        1072     1258     +186     
==========================================
+ Hits          445      490      +45     
- Misses        627      768     +141     
Flag Coverage Δ
unit-tests 38.96% <24.22%> (-2.56%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dark0dave
Copy link
Contributor Author

dark0dave commented Apr 14, 2024

@orhun I have tried to stick as close as I can to your github integration. Fortunately the gitlab api works in a very similar way to the github one.

Please tell me if you think this is the right approach.

@orhun
Copy link
Owner

orhun commented Apr 14, 2024

Wow, this is great! Thanks for making it happen <3

Is it actually functional now? Can we add some tests (fixtures) to verify the behavior?

@dark0dave
Copy link
Contributor Author

dark0dave commented Apr 14, 2024

Wow, this is great! Thanks for making it happen <3

Is it actually functional now? Can we add some tests (fixtures) to verify the behavior?

Yes I will add some tests in a bit!

Glad to see you happy for it!

@dark0dave dark0dave marked this pull request as ready for review April 14, 2024 21:28
@dark0dave
Copy link
Contributor Author

dark0dave commented Apr 14, 2024

@orhun I have a working integration test 🎉

Some slight bugs remain but we are almost there

@dark0dave
Copy link
Contributor Author

@orhun this is now complete, with tests and integration tests. I would appreciate it if you could look over it.

@dark0dave
Copy link
Contributor Author

@orhun let me know if you need anything more for me to merge this. Thank you!

@orhun
Copy link
Owner

orhun commented Apr 20, 2024

Thank you @dark0dave - this is pretty exciting. I will add a review soon!

@dark0dave
Copy link
Contributor Author

@orhun I have significantly reduced the fields in the gitlab merge request struct as requested.

If you would kindly take another look, thank you!

Signed-off-by: dark0dave <dark0dave@mykolab.com>
* feat(update): Remove vim by @dark0dave

## New Contributors
* @dark0dave made their first contribution in #
Copy link
Owner

Choose a reason for hiding this comment

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

Any idea why the MR number is empty? Maybe we can avoid this by checking it in the template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its because I am skipping merge commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add back in

@orhun
Copy link
Owner

orhun commented Apr 28, 2024

Hey, all looks good! One thing that I want to do before moving on with this is refactoring the code to avoid duplications between GitHub and GitLab integration. This will also make it easier to add other remotes in the future.

My plan is to introduce a trait (maybe called GitRemote or RemoteCommit, RemotePullRequest) then implement it over GitHub and GitLab types. Then we can move over these modules to remote/ and use generic methods to handle things. I'm talking off the top of my head now but that's roughly how it should be done I think.

I can get to this this week, but let me know what you think of it and possibly want to take a stab! Sorry it took a bit of time to get this PR in - due to the fact that it was a great surprise 😄

@dark0dave
Copy link
Contributor Author

Hey, all looks good! One thing that I want to do before moving on with this is refactoring the code to avoid duplications between GitHub and GitLab integration. This will also make it easier to add other remotes in the future.

My plan is to introduce a trait (maybe called GitRemote or RemoteCommit, RemotePullRequest) then implement it over GitHub and GitLab types. Then we can move over these modules to remote/ and use generic methods to handle things. I'm talking off the top of my head now but that's roughly how it should be done I think.

I can get to this this week, but let me know what you think of it and possibly want to take a stab! Sorry it took a bit of time to get this PR in - due to the fact that it was a great surprise 😄

Yeah I was thinking about that, the whole time I was doing this. It would make it easier for us to support more providers in the future. I was thinking we could do it as an improvement (ie annother PR). But happy to do it to this one if you think thats appropriate.

@orhun
Copy link
Owner

orhun commented May 11, 2024

Feel free to do it in this PR! I think I will also have some time this week, but feel free to beat me to it!

@orhun orhun mentioned this pull request May 24, 2024
12 tasks
@orhun
Copy link
Owner

orhun commented May 24, 2024

Hey @dark0dave! I made the changes in another PR since I don't have access to push to this PR: #654

I think the only thing left is to update the documentation and then release! 🚀

@orhun orhun closed this in #654 May 25, 2024
@orhun
Copy link
Owner

orhun commented May 25, 2024

@dark0dave thank you a lot once again! 💖

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.

None yet

4 participants