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

Drop git ls-files in gemspec #5

Merged
merged 1 commit into from
Jun 26, 2020
Merged

Conversation

utkarsh2102
Copy link
Contributor

@utkarsh2102 utkarsh2102 commented Jun 24, 2020

Hi @pboling,

Thanks for working on this! ❤️
However, while maintaining this in Debian, we found that this library relies on git to list the files which could be done via pure Ruby alternative -- which is what this PR does.

As an addition, this PR makes sure that this gem only ships the required files to the end users and not other things which are not needed by them 🚀

Also, added rubocop-packaging as a development_dependency which will ensure the best practices.
Here's what it shows us:

➜  rspec-stubbed_env git:(master)  rubocop --only Packaging

Offenses:

rspec-stubbed_env.gemspec:25:5: C: Packaging/GemspecGit: Avoid using git to produce lists of files. Downstreams
often need to build your package in an environment that does not have git (on purpose). Use some pure Ruby
alternative, like Dir or Dir.glob.
    `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) }
    ^^^^^^^^^^^^^^^^^

And this PR fixes the same, which helps us in shipping this in Debian! 🎉
Hope this would make sense and you'll be open to this change 💯

Signed-off-by: Utkarsh Gupta <utkarsh@debian.org>

Signed-off-by: Utkarsh Gupta <utkarsh@debian.org>
@pboling
Copy link
Owner

pboling commented Jun 26, 2020

@utkarsh2102 First of all... I love your changes. Second, ship this in debian?! Third, I love Debian. Fourth, how /why did you find this, and how/why is it useful to Debian?

Copy link
Owner

@pboling pboling left a comment

Choose a reason for hiding this comment

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

LGTM

@pboling pboling merged commit 710cb32 into pboling:master Jun 26, 2020
@utkarsh2102 utkarsh2102 deleted the drop-git branch June 26, 2020 21:06
@utkarsh2102
Copy link
Contributor Author

Hi @pboling 👋

First of all... I love your changes.

Thank you so much! ❤️

Second, ship this in debian?!

Hehe, yes! So we maintain oauth2 in Debian (as a Ruby package) which now depends on this awesome library! 🔥
So we packaged this, too (as all dependencies of a package should be packaged, too)!
This mean, you can do: apt install ruby-rspec-stubbed-env on a Debian system to install this gem (alternative to gem install rspec-stubbed_env, but system-wide 😄)

Third, I love Debian.

That is awesome! And Debian loves you ❤️
Thanks for your awesome work on this and oauth2 (and other libraries that you maintain!) 🔥

Fourth, how /why did you find this, and how/why is it useful to Debian?

So in Debian, we maintain a lot of applications and libraries, one of them is oauth (and now, oauth2).
Since oauth2 started depending on this, we packaged this library for Debian, too! 😄

I'll be happy to answer more of your questions if you have more!? \o/

@pboling
Copy link
Owner

pboling commented Jul 11, 2020

@utkarsh2102 So very cool. I am working on releases. 💯

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