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

Document some exported Sys CPU stuff #31204

Merged
merged 5 commits into from
Aug 6, 2019
Merged

Document some exported Sys CPU stuff #31204

merged 5 commits into from
Aug 6, 2019

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Feb 28, 2019

Can't help but attack #26919 when I should be doing other things...

@kshyatt kshyatt added the docs This change adds or pertains to documentation label Feb 28, 2019
@kshyatt kshyatt requested a review from vtjnash February 28, 2019 17:56
@StefanKarpinski
Copy link
Member

You are the best, @kshyatt.

base/sysinfo.jl Outdated Show resolved Hide resolved
base/sysinfo.jl Outdated Show resolved Hide resolved

Print a summary of information about each processor in `cpu` gathered from [`cpu_info](@ref). This is generally:
- The number (e.g. CPU 3 on a system with 10 total)
- The CPU model
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps put this first since it is the first thing to show

base/sysinfo.jl Outdated Show resolved Hide resolved
"""
Sys.cpu_info()

Gather information about all CPUs in the system through a syscall using LibUV.
Copy link
Member

Choose a reason for hiding this comment

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

libuv is usually in lowercase but I am not sure it is needed in the docstring at all.

We dont have any documentation about the type of struct getting returned CPUinfo, so there isn't really any useful thing you can do with the return value here. Arguably this should just not be exported (which is breaking).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we doc it for now then remove the export in the future?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Gather information about all CPUs in the system through a syscall using LibUV.
Gather information about the logical CPU cores in the system. This includes:
- The model name
- The clock speed
- The amount of time spent in `user`, `nice`, `sys`, `idle`, and `irq` modes

(and drop the corresponding list from Sys.cpu_summary)

doc/src/base/base.md Outdated Show resolved Hide resolved
KristofferC and others added 2 commits March 2, 2019 08:52
@kshyatt
Copy link
Contributor Author

kshyatt commented Mar 11, 2019

@KristofferC did you have any thoughts about the export issue and the explanation about "e.g."?

@fredrikekre
Copy link
Member

Since CI failed during doc-build on this PR on previous commits it would be good to not ci-skip and run CI before merging.

@kshyatt
Copy link
Contributor Author

kshyatt commented Mar 13, 2019

@fredrikekre my plan was to squash-and-merge and force push manually with no ci skipping once we settle on things

"""
Sys.cpu_info()

Gather information about all CPUs in the system through a syscall using LibUV.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Gather information about all CPUs in the system through a syscall using LibUV.
Gather information about the logical CPU cores in the system. This includes:
- The model name
- The clock speed
- The amount of time spent in `user`, `nice`, `sys`, `idle`, and `irq` modes

(and drop the corresponding list from Sys.cpu_summary)

@StefanKarpinski StefanKarpinski added the forget me not PRs that one wants to make sure aren't forgotten label Aug 6, 2019
@StefanKarpinski StefanKarpinski merged commit 7f2922e into master Aug 6, 2019
@delete-merged-branch delete-merged-branch bot deleted the ksh/doccpus branch August 6, 2019 03:27
staticfloat added a commit that referenced this pull request Aug 6, 2019
staticfloat added a commit that referenced this pull request Aug 6, 2019
Revert "Document some exported Sys CPU stuff (#31204)"
@simeonschaub simeonschaub removed the forget me not PRs that one wants to make sure aren't forgotten label May 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants