-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor code #4
Conversation
gemfile_lock_content: gemfile_lock_content || FileContent.for('Gemfile.lock'), | ||
bundler_audit_config_content: bundler_audit_config_content || FileContent.for('.bundler-audit.yml') | ||
).health_score | ||
def scan(**opts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will enable us to support command-line arguments for all configuration options later on.
def audit_db_stale? | ||
((Time.now - Bundler::Audit::Database.new.last_updated_at) / ONE_DAY) > 1.0 | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic was changed for this one to update the DB if it's stale more than one day instead of previously more than seven.
|
||
MAJOR_VERSION_PENALTY = 1 | ||
NEW_VERSIONS_SEVERITY = 1.07 | ||
SEGMENT_SEVERITIES = [1.7, 1.15, 1.01].freeze |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, this constant had values [1.7, 1.15, 1.01, 1.005]
. In practice, the last segment isn't very useful. For example, comparing versions v7.0.0 and v7.0.0.1 will not cause a health score difference because the former version only has three segments, so comparison only makes sense if you have versions like v7.0.0.1 and v7.0.0.2.
Because of that, and the fact that Semver doesn't define a fourth segment and its meaning is defined by dependency owners (like Rails, which uses it for security releases), I've decided to remove it. In the case of Rails, health score will still be impacted by advisories because v7.0.0.1 will have an advisory that v7.0.0.2 won't.
return 100 if up_to_date? | ||
return 1.0 if up_to_date? | ||
|
||
score = 100 | ||
score = 1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiplying by 100 happens in GemfileHealthScore now, at the very end of health score calculation.
@@ -8,23 +8,29 @@ class GemVersions | |||
def initialize(dependency_names, spec_type:) | |||
@dependency_names = dependency_names.to_set | |||
@spec_type = spec_type | |||
@gem_versions = Hash.new { |h, k| h[k] = [] } | |||
@gem_versions = Hash.new { |h, k| h[k] = Set.new } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to a set to remove potential version duplicates.
Refactors code to support upcoming features (CLI options, ruby versions impacting health score).
Notable changes:
DependencyContext
andCalculationContext
classes to hold business logic for dependencies and variables impacting the health score, respectivelyCodebaseHealthScore
class (replaced by direct usage ofGemfileHealthScore
)Tempfile
logic to use blocks so files are deleted automatically (seeDependencyContext
file) instead of being handled manually