Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

Move to gemspec #17

Closed
wants to merge 6 commits into from
Closed

Move to gemspec #17

wants to merge 6 commits into from

Conversation

lukaso
Copy link
Contributor

@lukaso lukaso commented Oct 2, 2019

I like this version of the version gemspec better, but need to be sure it flies with dependabot.

@lukaso lukaso requested a review from pvdb October 2, 2019 11:08
@lukaso
Copy link
Contributor Author

lukaso commented Oct 2, 2019

$ bundle
Using flip_fab 1.0.6 from source at `.`
Bundle complete! 7 Gemfile dependencies, 25 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
$ GEM_PRE_RELEASE=hacking bundle
Fetching gem metadata from https://rubygems.org/..........
Resolving dependencies...
Using flip_fab 1.0.6.hacking (was 1.0.6) from source at `.`
Bundle complete! 7 Gemfile dependencies, 25 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
$ GEM_PRE_RELEASE= bundle
Fetching gem metadata from https://rubygems.org/..........
Resolving dependencies...
Using flip_fab 1.0.6 (was 1.0.6.hacking) from source at `.`
Bundle complete! 7 Gemfile dependencies, 25 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
$ GEM_PRE_RELEASE='   ' bundle
Using flip_fab 1.0.6 from source at `.`
Bundle complete! 7 Gemfile dependencies, 25 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.

@pvdb
Copy link
Contributor

pvdb commented Oct 2, 2019

Hmm, not sure TBH, as per #16 (comment) ...

$ GEM_PRE_RELEASE=blegga ruby -Ilib -r 'flip_fab/version' -r bundler -e '
puts format("gem itself: %s", FlipFab::VERSION);
puts format("gemspec: %s", Bundler.load_gemspec("flip_fab.gemspec").version)'
gem itself: 1.0.6
gemspec: 1.0.6.blegga
$ _

Which one is it... 1.0.6 or 1.0.6.blegga?

Personally, I prefer to have all version-related logic nicely encapsulated in the version file (and as a result also avoid the highlighted discrepancy) ... but could be convinced otherwise, I guess 😃

@pvdb
Copy link
Contributor

pvdb commented Oct 2, 2019

[...] but need to be sure it flies with dependabot.

Not just today, but also going forward: if Dependabot makes any future changes to its "gemspec sanitizer" which our gemspec munging is incompatible with, we need to refactor our 40+ repos all over again.

@lukaso
Copy link
Contributor Author

lukaso commented Oct 2, 2019

[...] but need to be sure it flies with dependabot.

Not just today, but also going forward: if Dependabot makes any future changes to its "gemspec sanitizer" which our gemspec munging is incompatible with, we need to refactor our 40+ repos all over again.

Sure, but same is true of bumping. What we're doing version wise is, frankly, hacky. The correct way to do this is probably to actually change the version that is built, via a PR, right? (That's the correct way of tackling this, not sure it works for us though.)

@pvdb
Copy link
Contributor

pvdb commented Oct 2, 2019

Rubocop not too happy about this version:

flip_fab.gemspec:31:3: C: Gemspec/DuplicatedAssignment: version= method calls already given on line 8 of the gemspec.

(style guide for Gemspec/DuplicatedAssignment)

@lukaso
Copy link
Contributor Author

lukaso commented Oct 2, 2019

It might just be a dependabot bug.

See this from dependabot's own gemspec:

Gem::Specification.new do |spec|
  common_gemspec =
    Bundler.load_gemspec_uncached("../common/dependabot-common.gemspec")

  spec.name         = "dependabot-bundler"
  spec.summary      = "Ruby (bundler) support for dependabot"
  spec.version      = common_gemspec.version

@lukaso
Copy link
Contributor Author

lukaso commented Oct 2, 2019

Here's my breaking test on dependabot:

gemspec_santizer_spec.rb

      context "with a combination of version constant and variables" do
        let(:content) { 
          %q(gem_version = if false
              Example::VERSION
            else
              "#{Example::VERSION}.test_version"
            end
            Spec.new { |s| s.version = gem_version })
        }
        it { is_expected.to eq( %q(gem_version = if false
              "1.5.0"
            else
              "#{"1.5.0"}.test_version"
            end
            Spec.new { |s| s.version = "1.5.0" }) ) }
      end

Which outputs:

Failures:

  1) Dependabot::Bundler::FileUpdater::GemspecSanitizer#rewrite version assignment with a combination of version constant and variables should eq "gem_version = if false\n              \"1.5.0\"\n            else\n              \"\#{\"1.5.0\"}.test_version\"\n            end\n            Spec.new { |s| s.version = \"1.5.0\" }"
     Failure/Error:
       it { is_expected.to eq( %q(gem_version = if false
             "1.5.0"
           else
             "#{"1.5.0"}.test_version"
           end
           Spec.new { |s| s.version = "1.5.0" }) ) }

       expected: "gem_version = if false\n              \"1.5.0\"\n            else\n              \"\#{\"1.5.0\"}.test_version\"\n            end\n            Spec.new { |s| s.version = \"1.5.0\" }"
            got: "gem_version = if false\n              Example::VERSION\n            else\n              \"\#{\"1.5.0\"}.test_version\"\n            end\n            Spec.new { |s| s.version = \"1.5.0\" }"

       (compared using ==)

       Diff:
       @@ -1,5 +1,5 @@
        gem_version = if false
       -              "1.5.0"
       +              Example::VERSION
                    else
                      "#{"1.5.0"}.test_version"
                    end

     # ./spec/dependabot/bundler/file_updater/gemspec_sanitizer_spec.rb:230:in `block (5 levels) in <top (required)>'

@lukaso
Copy link
Contributor Author

lukaso commented Oct 2, 2019

@lukaso
Copy link
Contributor Author

lukaso commented Oct 2, 2019

Rubocop not too happy about this version:

flip_fab.gemspec:31:3: C: Gemspec/DuplicatedAssignment: version= method calls already given on line 8 of the gemspec.

(style guide for Gemspec/DuplicatedAssignment)

We have a different rubocop ignore :)

@pvdb
Copy link
Contributor

pvdb commented Jan 22, 2020

Superseded by #21 ... closing PR.

@pvdb pvdb closed this Jan 22, 2020
@KayliBrownstein KayliBrownstein deleted the move_to_gemspec branch August 2, 2023 18:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants