Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add a FreeBSD impl #2

Merged
merged 9 commits into from
Jan 19, 2022
Merged

Add a FreeBSD impl #2

merged 9 commits into from
Jan 19, 2022

Conversation

Freaky
Copy link
Collaborator

@Freaky Freaky commented Jan 12, 2022

Started this before I saw @adriaandegroot's work, oh well.

It could do with some tidying, but it appears to work.

@adriaandegroot
Copy link
Contributor

You fixed the shebang, that's something I totally missed, and I didn't know about py-sysctl (nor, in my last PR, about time.monotonic_ns() so I'm going to steal that).

@jimsalterjrs
Copy link
Owner

Do we have any kind of consensus on whose patches I should be merging? I haven't looked super closely yet, but it sounds like you're both taking separate approaches to the same goal.

Linux code now massages the /proc format into a dict, avoiding half a
dozen conditionals per field per dataset.
Replace the listdir and startswith calls with a simple glob.  Add a
comment describing the format of these files.

Also fix the conditional in the dict comprehension generating that
passed to Dataset(), which is both an incorrect number and against the
wrong field.
@adriaandegroot
Copy link
Contributor

Do we have any kind of consensus on whose patches I should be merging? I haven't looked super closely yet, but it sounds like you're both taking separate approaches to the same goal.

I think @Freaky is putting a little more effort here, and has covered several edge cases better. Looking at our different approaches, I'm into class hierarchies and defining code for all targets, and then subbing in types and functions as appropriate; he defines only things for the current platform, with no class hierarchy. To me, his code looks un-idiomatically terse, but cool (like the way to get *fields out of the contents of one of those objset files).

I'd suggest taking this implementation and closing #3

@Freaky
Copy link
Collaborator Author

Freaky commented Jan 15, 2022

I mostly use Ruby for scripting, probably helps explain the style - hopefully it's not too terse, I'm not trying to make it compact for the sake of it.

@adriaandegroot certainly has the more conservative approach, which is no bad thing if it's easier for others to understand/maintain.

@jimsalterjrs jimsalterjrs merged commit eb1ff60 into jimsalterjrs:main Jan 19, 2022
@jimsalterjrs
Copy link
Owner

Since @adriaandegroot is the competitor and likes this one better, I'm going with this one. Thanks both!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants