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

Use git ls-files -z in gemspec #96

Closed
wants to merge 2 commits into from
Closed

Conversation

pocke
Copy link
Contributor

@pocke pocke commented Jan 9, 2020

Currently, the gemspec has a warning about $INPUT_RECORD_SEPARATOR.

$ bundle exec ruby -w -rrainbow -e 
/home/pocke/ghq/github.com/sickill/rainbow/rainbow.gemspec:18: warning: global variable `$INPUT_RECORD_SEPARATOR' not initialized

This pull request suppresses the warning.
I think using -z option and split("\x0") is better than the current.
bundle gem's boilerplate also has the style.


RuboCop's new cop added an offense. And I got warnings about .rubocop.yml.
So this pull request also suppresses the warning.

@olleolleolle
Copy link
Collaborator

  • The English library, require 'English' defines this global variable
  • Its shorter name is $/, which is a special variable that delimits records, normally a String which is a '\n'
  • RuboCop has a cop which marks special variables undesirable in code

We could:

  • Require the standard library English to enable these longer words, and do that in the gemspec or in the version.rb which is already included (perhaps a little cryptic)
  • Use the shorter name of the variable (and disable the RuboCop warning - but this is not desirable for the other "special variables" we'd like us not to use)

Example in IRB:

image

@olleolleolle
Copy link
Collaborator

olleolleolle commented Jan 9, 2020

We can also simplify away the git need:

spec.files = Dir['lib/**/*.rb'] + %w[Changelog.md README.markdown LICENSE]

@pocke
Copy link
Contributor Author

pocke commented Jan 9, 2020

Yes, $INPUT_RECORD_SEPARATOR is an alias of $/ and added by English.

I think -z is more appropriate than using $/ or the aliases. Because -z treats file path that contains newlines.

Bundler also has been changed from $/ to -z.
rubygems/bundler@ec9165f#diff-bfb3596c9458be429d82bf6b31f82364L16

However, we probably never add a file that contains a newline. But I think -z is the new standard style of gemspec, so we can follow it.

By the way, Dir['lib/**/*.rb'] seems reasonable. But I usually do not see this style in gemspec, so it's possible to have any problems. I guess it probably works well.

Thanks for your comment!

@olleolleolle
Copy link
Collaborator

Superseded by #97 - thanks for raising this issue, @pocke - and for fixing the RuboCop build failures! 🍏 💚 GREEN AGAIN!

@olleolleolle
Copy link
Collaborator

Am closing this with a thanks!

@pocke pocke deleted the fix-gemspec branch January 9, 2020 13:11
@pocke
Copy link
Contributor Author

pocke commented Jan 9, 2020

Thanks!

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

2 participants