Conversation
As we still support Ruby < 1.9
agrim123
changed the title
agrim123
force-pushed
the
bundle-canonical
branch
from
a650b35 to
ed51bf6
Compare
segiddins
left a comment
segiddins
left a comment
There was a problem hiding this comment.
This will need an RFC before we can merge, but I think this is a cool start!
| Prettifies the Gemfile by giving it consistent ordering and formatting. | ||
| D | ||
| method_option "view", :type => :boolean | ||
| def canonical |
There was a problem hiding this comment.
we''l need to come up with a better name for this, but we can do so in the RFC
There was a problem hiding this comment.
Yeah, I was thinking maybe prettify or something on those lines.
| # frozen_string_literal: true | ||
|
|
||
| module Bundler | ||
| class Gemfile |
There was a problem hiding this comment.
this class definitely should be named something different GemfileRewriter or something
There was a problem hiding this comment.
I don't think GemfileRewriterwould suit because we are not changing gemfile anywhere. We are extracting things out of it. Need to think of something else 🤔
spec/bundler/gemfile_spec.rb
Outdated
| gem "rspec" | ||
| end | ||
| E | ||
| expect(subject.groups_wise(a[2][1], a[2][0]).join("\n").gsub(/\n{3,}/, "\n\n")).to eq(strip_whitespace(expected)) |
There was a problem hiding this comment.
these tests are very hard to read -- maybe extract into some variable names?
There was a problem hiding this comment.
My bad. Fixing them.
|
I'm unsure about this change, |
|
@deivid-rodriguez I agree but we are basically making the Gemfile uniform and if required with the summary of the gems. I don't think rubocop provides this. |
|
This is what |
|
This seems to hijack this PR 😅. |
|
I am still in favor of bundler having this functionality built in -- it behaves fundamentally differently from rubocop, since it operates on the semantic content of the gemfile, rather than just the ruby source |
|
@segiddins Can you explain the difference a bit more? From the PR description, "To prettify the gemfile", "Give it consistent ordering and formatting", to me it looks very similar to what |
|
RuboCop doesn’t actually eval the gemfile, so it won’t know the underlying contents, such as if Ruby code is used in conditionals |
|
Mmm, I think I get what you mean. database_gem = "pg"
gem database_gem
gem "decidim"
I'm still hesitant about whether this is worth maintaining... Any more opinions? |
|
@deivid-rodriguez comments on the RFC appreciated :) |
|
Ooops, sorry for commenting in the wrong place. I'll move my hesitations there 😄 |
|
Personally, I don't see much benefits (if at all) of adding and using this. Style checking and formatting should be done by external linters, such as rubocop. And for short summaries for gems - it's pointless to add summaries for all gems in Gemfile, as this is intended for. Usually people annotate only a couple of gems in Gemfile and it is not that hard to do it manually. |
|
Closing this as per rubygems/rfcs#15 (comment). Thanks for the work and discussion in any case 🙏. |
4 participants
Basic Idea
Usage
By default changes the gemfile and on using
--viewoption displays what the gemfile would look like without altering it.Specs are failing. Working on them