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

Add lockfile #32

Merged
merged 2 commits into from
Jan 8, 2024
Merged

Add lockfile #32

merged 2 commits into from
Jan 8, 2024

Conversation

Irving-Betterment
Copy link
Contributor

@Irving-Betterment Irving-Betterment commented Dec 27, 2023

Summary of changes:
Commit lockfile previously ignored (see https://bundler.io/guides/faq.html#using-gemfiles-inside-gems).

/task https://app.asana.com/0/0/1206244989881229/f

@Irving-Betterment Irving-Betterment force-pushed the icaro/lockfiles_commit branch 2 times, most recently from 24ffa5a to 97e5bde Compare December 27, 2023 22:54
@Irving-Betterment Irving-Betterment marked this pull request as draft December 28, 2023 20:46
@Irving-Betterment Irving-Betterment force-pushed the icaro/lockfiles_commit branch 6 times, most recently from 9c16b46 to 26b4274 Compare January 5, 2024 22:13
@Irving-Betterment Irving-Betterment changed the title Add lockfile(s) Add lockfile Jan 5, 2024
@@ -4,7 +4,6 @@
.DS_Store
.rvmrc
/coverage
Gemfile.lock
gemfiles/*.lock
Copy link
Contributor Author

@Irving-Betterment Irving-Betterment Jan 5, 2024

Choose a reason for hiding this comment

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

I kept appraisal lockfiles ignored because with the current Ruby version support bracket (2.6 ... 3.2) some dependencies like nokogiri cannot be locked due not having enough broad support (1, 2). Therefore CI test matrix fails for some of these incompatible combinations.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we get much of the benefits without this, but I'm approving this PR because at least the primary gemfile will allow for a consistent experience for someone who pulls down the repo.

Copy link
Contributor Author

@Irving-Betterment Irving-Betterment Jan 8, 2024

Choose a reason for hiding this comment

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

@smudge indeed, starting with rubocop (lint job) not drifting due new cops available on each release (like the one I had to adjust myself here).

@@ -34,7 +34,7 @@ def run!
attr_reader :jobs

def emit_metric!(metric)
send("#{metric}_grouped").reverse_merge(default_results).each do |(priority, queue), value|
send(:"#{metric}_grouped").reverse_merge(default_results).each do |(priority, queue), value|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Address rubocop offense that cropped up.

@@ -16,3 +16,6 @@ AllCops:

Rails/EnvironmentVariableAccess:
Enabled: false

RSpec/IndexedLet:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rubocop started to lint this cop, which I disabled as spec subjects have this pattern.

@Irving-Betterment Irving-Betterment marked this pull request as ready for review January 5, 2024 23:46
Copy link
Member

@smudge smudge left a comment

Choose a reason for hiding this comment

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

domain LGTM && platform LGTM

@smudge smudge merged commit 737f2fd into main Jan 8, 2024
41 checks passed
@smudge smudge deleted the icaro/lockfiles_commit branch January 8, 2024 17:58
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.

2 participants