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

Warn if user is not using a supported platform #7381

Merged
merged 19 commits into from
Oct 2, 2020

Conversation

dv8silencer
Copy link
Contributor

@dv8silencer dv8silencer commented Sep 30, 2020

What type of PR is this?

Feature

What does this PR do? Why is it needed?
Early in the invocation of the clients, this PR will make it so that there is warn-level output if the user's platform is not supported.

Which issues(s) does this PR fix?

Fixes #5797

Other notes for review

@dv8silencer dv8silencer requested a review from a team as a code owner September 30, 2020 03:42
@dv8silencer dv8silencer marked this pull request as draft September 30, 2020 03:42
@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

Merging #7381 into master will increase coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7381      +/-   ##
==========================================
+ Coverage   60.07%   60.23%   +0.15%     
==========================================
  Files         419      419              
  Lines       30456    30649     +193     
==========================================
+ Hits        18297    18461     +164     
- Misses       9146     9154       +8     
- Partials     3013     3034      +21     

@dv8silencer dv8silencer marked this pull request as ready for review October 1, 2020 01:41
@dv8silencer dv8silencer changed the title Warn if user is not using a supported platform - Work In Progress (Draft) Warn if user is not using a supported platform Oct 1, 2020
shared/prereq/prereq.go Outdated Show resolved Hide resolved
shared/prereq/prereq.go Outdated Show resolved Hide resolved
beacon-chain/node/node.go Outdated Show resolved Hide resolved
Copy link
Contributor

@farazdagi farazdagi left a comment

Choose a reason for hiding this comment

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

Looks great, the only thing that seems to be off is stopping node execution if there is some issue in detecting the OS type/version.

The scope of this PR should be limited to providing warning/guidance to user, not amending ability to run nodes.

@dv8silencer dv8silencer requested a review from farazdagi October 1, 2020 22:50
Copy link
Contributor

@farazdagi farazdagi left a comment

Choose a reason for hiding this comment

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

Amazing work, thank you!

@farazdagi farazdagi merged commit 7d9a706 into prysmaticlabs:master Oct 2, 2020
@dv8silencer dv8silencer deleted the dv8-i5797 branch October 2, 2020 13:05
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.

Check OS / pre-req's and fail fast if not met
2 participants