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

Remove Gemfile.lock #1040

Closed
wants to merge 1 commit into from
Closed

Remove Gemfile.lock #1040

wants to merge 1 commit into from

Conversation

colszowka
Copy link
Collaborator

  • This adds an explicit ref to the commit we use for apparition from git, as the gem is still unreleased (see make Page.frameNavigated accept additional keywords twalpole/apparition#79 and test: Use unreleased version of apparition #1013)
  • Removes Gemfile.lock - not sure why we have this in the source control but for gems we shouldn't have it committed, but rather the gemspec itself should declare strict dependencies in order to ensure that when CI runs (with newest allowed dependency versions) we confirm they work
  • The explicit commit ref is to ensure we don't install unwanted updates to the gem beyond the commit we specified, now that Gemfile.lock is gone

@colszowka
Copy link
Collaborator Author

Hmm, looks like CI doesn't like this, I will have to revisit this in later on then because my main intention right now is to cut a new release with the Ruby 3.2 fix from #1035 as well as coverage for eval support from #1037

@deivid-rodriguez
Copy link
Collaborator

For what it's worth, the rationale is here: #770. I believe the CI failures that appeared here after unlocking all dependencies are also a good example of the motivations.

This is a very personal choice, and I'm no longer active maintaining this gem and Tobias does not seem too active either, so of course feel free to adapt this to your favorite practices. But I thought I'd at least point to the motivation :)

@colszowka
Copy link
Collaborator Author

@deivid-rodriguez Thanks a lot for giving some context - this should then be investigated more closely for sure, and not done on a whim. Regardless though it seems CI is having issues unrelated to this, as another PR I made with just changelog entries and a version bump also runs into similar issues it seems - #1041, whereas i.e. the pipeline for #1037 worked just fine recently and also passed on main after merge 🤷

I'll leave this one around for the time being though, don't want to make a mess out of prior decisions over here

@colszowka
Copy link
Collaborator Author

Alright at least the bundling issues on gh actions I was able to resolve, it seems something was off with the bundle cache. Along the way I also updated the Gemfile to latest bundler as well and now the CI is also running on 761956b #1041

@deivid-rodriguez
Copy link
Collaborator

deivid-rodriguez commented Dec 23, 2022

I think the issues are different.

The ones here are some compatibility issues with rspec-mocks (keyword arguments), I recall them because I went through them at some point in other place. They happen because with a Gemfile.lock you're locked to the working 3.10.2 version while without a Gemfile.lock file you use the latest version which probably introduces those issues.

The problem in the other PR does not seem related to dependencies but to Bundler itself 🤔

@deivid-rodriguez
Copy link
Collaborator

Oh, our messages crossed, glad you were able to resolve it!

@colszowka
Copy link
Collaborator Author

@deivid-rodriguez Thanks for your input, yeah, there seems to be something off with the CI there, but it's a bit mysterious. Now at least CI started properly, but it's failing with other strange errors, on a PR that merely adds a changelog and bumps the gem version 🤷 I guess I'll have to keep debugging 🤷

@deivid-rodriguez
Copy link
Collaborator

There's actually other Gemfile.lock files in the repo for tests. And those use gem version. Since CI is setup in frozen mode, and the version has changed, bundling those other lockfiles causes source control changes updating the version and frozen mode does not like that.

I think you can switch to each Gemfile.lock folder, run bundle lock and that should update the locked version of simplecov. Committing those changes should fix the CI I believe.

@colszowka
Copy link
Collaborator Author

Thanks, I'll take a look - I'll also redirect this discussion to #1041 (comment) since the last comments were actually about the problems with the CI over there anyway :)

@colszowka
Copy link
Collaborator Author

And closing this one as I'd have to dive into the topic more thoroughly as discussed above, and the Gemfile.lock is in there intentionally right now

@colszowka colszowka closed this Dec 23, 2022
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