elastic / elastic/logstash

refactor LogStash::PluginManager::Update#validate_major_version

Open
#3,744 18 comments 0 reactions 1 assignee Claimed by @purbon View on GitHub
code cleanup
Dominant language
Java
Stars
14.9k
Forks
3.5k
Avg merge
19h 14m
Merged PRs (30d)
63

Description

this method https://github.com/elastic/logstash/blob/028d76497ccef3a8d11a3528cdf6b99e8d10f070/lib/pluginmanager/update.rb#L70-L80 introduced by PR #3423 has a few issues:
- this method, like all other "lower level" methods which directly interact with Rubygems should be in the `util.rb` module.
- we should avoid, as much as possible, lazy require calls.
- I don't think we have prior-art for introducing an interactive Y/N in the logstash UI and I don't think this has been properly discussed in terms of strategy. what about automated updates? did we consider instead to introduce something like a `--force` options?
- the `validate_major_version` method name is confusing, you actually have to read the code to understand it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.