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

Add gitlab: git source shorthand #7449

Merged

Conversation

jgarber623
Copy link
Contributor

@jgarber623 jgarber623 commented Feb 1, 2024

First off, thank you for considering this Pull Request! I appreciate your time.

What was the end-user or developer problem that led to this PR?

In #7210, I noted the existence of the github: and bitbucket: shorthands for installing gems from Git sources. I proposed adding two new providers, gitlab: and codeberg:, for the notable Git services Gitlab and Codeberg. The proposal to add Gitlab was accepted.

The changes proposed in this PR would provide a small quality-of-life improvement for users with Ruby dependencies whose source repositories exist somewhere other than GitHub.

Resolves #7210.

What is your fix for the problem, implemented in this PR?

This PR adds gitlab: shorthand with similar functionality to the existing github: shorthand:

gem "gitlab-sdk", gitlab: "gitlab-org/analytics-section/product-analytics/gl-application-sdk-rb"

Merge Request URLs are also supported for each new shorthand:

gem "gitlab-sdk", gitlab: "https://gitlab.com/gitlab-org/analytics-section/product-analytics/gl-application-sdk-rb/-/merge_requests/27"

Make sure the following tasks are checked

Copy link

welcome bot commented Feb 1, 2024

Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack.

For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide

@deivid-rodriguez
Copy link
Member

Thanks!

I think in the related issue there was pretty much consensus on adding Gitlab. I think codeberg was more unclear, though. In particular, thresholds proposed by @indirect at #7210 (comment) would exclude it.

So we can proceed in 2 ways:

  • Add Gitlab only for now.
  • Wait a bit until we make a final decision.

Regarding the implementation, it's pretty confusing, but the tests you added would be for an implementation of the Bundler DSL that actually lives in RubyGems and it's not really used by Bundler at the moment. The proper place for these tests would be in bundler/spec/bundler/dsl_spec.rb.

@indirect
Copy link
Member

indirect commented Feb 1, 2024

I think we should definitely add GitLab. I am less clear on whether we should add Codeberg (and Sourcehut, and Beanstalk, and CodeCommit, and Cloud Source Repositories, and Launchpad, and Keybase, and Planio, and Tuleap). My main hesitation adding smaller repository sources is churn, having to constantly do work to update our shortcuts if URLs change or services shut down or move.

If we see consistent user requests for other sources, I am fine with adding them. So far I think we have only seen that for GitLab.

@deivid-rodriguez
Copy link
Member

@jgarber623 Could you limit this PR to adding support for the gitlab shorthand?

jgarber623 added a commit to jgarber623/rubygems that referenced this pull request Feb 3, 2024
Follow-up from PR comments:

> I think in the related issue there was pretty much consensus on adding
> Gitlab. I think codeberg was more unclear, though.

rubygems#7449 (comment)

> I think we should definitely add GitLab. I am less clear on whether we
> should add Codeberg (and Sourcehut, and Beanstalk, and CodeCommit, and
> Cloud Source Repositories, and Launchpad, and Keybase, and Planio,
> and Tuleap).

rubygems#7449 (comment)

This reverts commit 41b9156.
jgarber623 added a commit to jgarber623/rubygems that referenced this pull request Feb 3, 2024
Response to feedback on PR rubygems#7449. The specs originally added in
91f3b2f weren't correct. This commit removes the erroneous specs and
adds relevant specs to the existing `Bundler:Dsl` examples.

The `gitlab:` gem source shortcut is modeled on the existing `github:`
shortcut, so the new specs mimic the existing examples.
@jgarber623
Copy link
Contributor Author

Thanks for the feedback, y'all! I've reverted the commit adding the Codeberg shortcut. Let me know if you'd rather avoid the revert commit and I can force push. Not sure which of GitHub's many merge approaches you prefer.

@deivid-rodriguez My apologies for that first pass at the tests. I've added a bunch of gitlab-specific ones in 98905e6, basing them on the existing github specs (since the two shortcuts are mostly the same). Let me know if there's anything else worth testing.

Thanks again and appreciate your consideration!

@deivid-rodriguez
Copy link
Member

No problem at all! Not your fault, it's a bit confusing 😅

Yes, if you can squash everything into one commit, I do prefer that 👍.

Thanks for the PR!

@jgarber623 jgarber623 force-pushed the add-new-git-source-shorthands branch 2 times, most recently from e52dfa2 to 9fa9e0b Compare February 6, 2024 02:42
@jgarber623
Copy link
Contributor Author

@deivid-rodriguez All set. Squashed the commits, brought my branch up-to-date with main, and force-pushed. Should be a nice clean merge.

Thanks again for the feedback!

@deivid-rodriguez deivid-rodriguez changed the title Add gitlab: and codeberg: Git source shorthands Add gitlab: git source shorthand Feb 7, 2024
Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Thank you!

@martinemde
Copy link
Member

martinemde commented Feb 7, 2024

Waiting for tests to be fixed. Please cleanup white space.

This new shorthand, similar to the existing `github:` shorthand, adds
support for Gitlab repositories with a notable difference. Gitlab
projects may be organized into projects and subprojects. An example
Ruby gem exists at:

https://gitlab.com/gitlab-org/analytics-section/product-analytics/gl-application-sdk-rb

With the new shorthand, a user may install this gem from its repository
by adding:

```ruby
gem "gitlab-sdk", gitlab: "gitlab-org/analytics-section/product-analytics/gl-application-sdk-rb"
```

As with the `github:` shorthand, a supplied string with no `/` will be
interpreted as `example/example`.

Also in keeping with the utility of the `github:` shorthand, the new
`gitlab:` shorthand also supports Merge Request URLs.

```ruby
gem "gitlab-sdk", gitlab: "https://gitlab.com/gitlab-org/analytics-section/product-analytics/gl-application-sdk-rb/-/merge_requests/27"
```

The `gitlab:` gem source shortcut is modeled on the existing `github:`
shortcut, so the new specs mimic the existing examples.
@jgarber623
Copy link
Contributor Author

Formatting issues addressed. 👍🏻

Both rspec ./spec/quality_spec.rb and rake rubocop are reporting all green.

@martinemde martinemde merged commit 6572919 into rubygems:master Feb 16, 2024
83 checks passed
@martinemde
Copy link
Member

martinemde commented Feb 16, 2024

Thanks @jgarber623 !

@jgarber623 jgarber623 deleted the add-new-git-source-shorthands branch February 16, 2024 14:01
deivid-rodriguez pushed a commit that referenced this pull request Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Codeberg and GitLab shorthands
5 participants