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

Exclude .gitignore from the package to support vendoring. #743

Merged

Conversation

iliabylich
Copy link
Collaborator

If .gitignore is a part of the .gem it's impossible to include source of
the locally installed parser (using bundle install --path ./vendor) because git
automatically excludes all gitignore-d files (including all parsers and lexer).

If .gitignore is a part of the .gem it's impossible to include source of
the locally installed parser (using bundle install --path ./vendor) because git
automatically excludes all gitignore-d files (including all parsers and lexer).
@iliabylich iliabylich merged commit f925b97 into whitequark:master Sep 24, 2020
@iliabylich iliabylich deleted the exclude-gitignore-from-the-package branch September 24, 2020 16:26
@@ -35,7 +35,10 @@ Gem::Specification.new do |spec|
lib/parser/ruby30.rb
lib/parser/macruby.rb
lib/parser/rubymotion.rb
) - %w(
.gitignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

@iliabylich OT: But thought triggered by your commit, I think parser should not use git at all to find packaged files.

See: mbj/mutant#1052 where I improved the packagability for debian and others.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wait, is there a debian package of parser? This field is used to build .gem file locally and push it to rubygems.org. I agree that it requires git locally to build it (which is kinda bad if you only to build a package), but at the same time it offers freedom of using any test files that are added to global .gitignore (I have files like test<1-10>.rb there).

I'd prefer to keep it as is if nobody needs it. Feel free to send a PR that fixes it though, I'll merge it (and I'll do cleanup of my local garbage 😄 ), but if it's just a "good practice" that nobody needs I'd like to ignore it 👀

Copy link
Collaborator

Choose a reason for hiding this comment

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

@utkarsh2102 feel free to weigh in here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Eee, hello 👋🏻

I am the maintainer of parser in Debian. It's actively maintained and is synced with the latest version! 😄
(so first of all, thanks for your work, really! 💯)

Yep, thanks @mbj for pointing this out, using git is a bit troublesome and it's best if avoided!
The complete rationale is here: https://docs.rubocop.org/rubocop-packaging/cops_packaging.html#gemspec-git-rationale

I'll be v happy to send a PR to drop git from gemspec! 🌮

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll be v happy to send a PR to drop git from gemspec! 🌮

Yes, please. I think {bin,lib}/**/*.rb pattern should be enough

Copy link
Collaborator

Choose a reason for hiding this comment

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

This will also drop the .y which are not needed for a bundled gem. Should be a nice size reduction.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, great! Will open a PR later today; completing my school homework for now! 😭

@utkarsh2102 utkarsh2102 mentioned this pull request Sep 25, 2020
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

3 participants