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.

Sign up for GitHub

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

Jump to bottom

Refactor OS detection. #561

Merged
merged 21 commits into from
Jan 28, 2020
Merged

Refactor OS detection. #561

merged 21 commits into from
Jan 28, 2020

Conversation

zenspider
Copy link
Contributor

@zenspider zenspider commented Jan 22, 2020 •
edited
Loading

master:

  758.00: flay total
   843.4: flog total
   843.4: flog/method average

final:

   38.00: flay total (95% reduction)
   616.3: flog total (25% reduction)
    38.5: flog/method average (95% reduction)

Signed-off-by: Ryan Davis zenspider@chef.io

@zenspider zenspider requested a review from a team as a code owner January 22, 2020 23:13
@ghost ghost requested review from clintoncwolfe and miah January 22, 2020 23:13
@chef-expeditor
Copy link
Contributor

Hello zenspider! Thanks for the pull request!

Here is what will happen next:

  1. Your PR will be reviewed by the maintainers.
  2. Possible Outcomes
    a. If everything looks good, one of them will approve it, and your PR will be merged.
    b. The maintainer may request follow-on work (e.g. code fix, linting, etc). We would encourage you to address this work in 2-3 business days to keep the conversation going and to get your contribution in sooner.
    c. Cases exist where a PR is neither aligned to Chef InSpec's product roadmap, or something the team can own or maintain long-term. In these cases, the maintainer will provide justification and close out the PR.

Thank you for contributing!

@zenspider zenspider force-pushed the zenspider/refactor/os branch 4 times, most recently from 6969bf2 to 97262b9 Compare January 23, 2020 02:44
@zenspider zenspider force-pushed the zenspider/refactor/os branch from ab84e93 to 9ff9956 Compare January 27, 2020 23:23
@zenspider
Refactor OS detection.
79378b1 
The first step is to just break it up into smaller chunks based on OS
family.

master:

  758.00: flay total
   843.4: flog total
   843.4: flog/method average

now:

  758.00: flay total
   966.8: flog total
   138.1: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored read_linux_lsb detection.
a08d9bf 
  646.00: flay total
   939.4: flog total
   134.2: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored unix_uname_s (BSD) detection.
3cd29e0 
  625.00: flay total
   904.2: flog total
   129.2: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored redhat detection and folded into register_lsb.
662c528 
  533.00: flay total
   882.5: flog total
   126.1: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Moved refactorings from lambdas to class methods.
8e80a3e 
  533.00: flay total
   920.6: flog total
    70.8: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored cisco detection.
f5022b1 
  591.00: flay total
   888.9: flog total
    68.4: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored json_cmd.
56dc838 
  357.00: flay total
   822.6: flog total
    68.6: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored register_path for solaris detection.
4c9d27b 
  392.00: flay total
   797.6: flog total
    66.5: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored Platform#detect.
e7582ca 
Why do we keep checking for nil and doing some third case when we're
NOT using trinary logic?

Also this method really should NOT be both a getter and setter.

  392.00: flay total
   797.6: flog total
    66.5: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored to register_os_or_file for suse detection.
a9b18bb 
Also got rid of `== true` checks on scanner. We don't care that it is
true, we want truthy.

  346.00: flay total
   788.5: flog total
    60.7: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored to set_from_uname.
325e2e8 
  274.00: flay total
   766.0: flog total
    58.9: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored to register_file_content for slackware and gentoo.
b62bfa6 
  228.00: flay total
   754.0: flog total
    53.9: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored to register_path_and_uname for arch and exherbo.
5a3ac5d 
  190.00: flay total
   743.7: flog total
    49.6: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored to register_path_regexp for smartos and nextentacore.
d481154 
  154.00: flay total
   738.1: flog total
    46.1: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Refactored register_lsb_or_content to allow for 2 regexps to fold in …
9106627 
…redhat.

  154.00: flay total
   719.6: flog total
    45.0: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Minor refactorings in the yocto detectors.
a23dd7b 
Mostly using local vars and regexps more cleanly.

  154.00: flay total
   712.9: flog total
    44.6: flog/method average

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Code cleanup of os.rb & os_common.rb
e6d0ccc 
Removed needless comments on already self-documenting blocks.
Refactored brocade_version.
Extracted redhatish version
Extracted local_windows?
Logical rewrites of overly complex logic.
Lots of `unless x.nil` -> `if !x.nil?` -> `if x`.
Switched a number of unused unix_file_contents to unix_file_exist? calls.

   38.00: flay total
   616.3: flog total
    38.5: flog/method average

Via walk-through with @miah.

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Found a bunch of cases that don't have tests. :(
f858295 
Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Fixed darwin detection to not read content of sw_vers.
6580798 
Use unix_uname_* and `sw_vers -buildVersion` for our data.

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Remove some of the clever and rename a lot of things.
6a20f64 
Removed all "clevar" method chains like:

  plat.name("raspbian").title("Raspbian Linux").in_family("debian").detect do ...

in favor of a simpler descriptive call:

  declare_instance("raspbian", "Raspbian Linux", "debian") do ...

Also added `declare_category` and `declare_family` to make things a
little more clear what they're for.

Renamed all `register_*` methods to `declare_*` methods.

Removed almost all rubocop magic comments.

The diff on this is nasty because of whitespace changes. Turn off
whitespace in github UI take make things easier. It's still gross, but
still.

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider
Rework the build pipeline.
5cf121b 
+ Separate out lint to its own stage.
+ Switch off of -stretch.
+ Switch 2.7 to final.
+ Default rake task is now just test.

Signed-off-by: Ryan Davis <zenspider@chef.io>
@zenspider zenspider force-pushed the zenspider/refactor/os branch from 9ff9956 to 5cf121b Compare January 28, 2020 00:04
@codeclimate
Copy link

codeclimate bot commented Jan 28, 2020

Code Climate has analyzed commit 5cf121b and detected 252 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 36
Bug Risk 27
Style 189

View more on Code Climate.

@zenspider zenspider merged commit c22dae9 into master Jan 28, 2020
@chef-expeditor chef-expeditor bot deleted the zenspider/refactor/os branch January 28, 2020 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clintoncwolfe clintoncwolfe clintoncwolfe approved these changes

@miah miah miah approved these changes

Assignees

@zenspider zenspider

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zenspider @miah @clintoncwolfe