Skip to content

Implemented chcpu#234

Merged
sylvestre merged 2 commits intouutils:mainfrom
mdcssw:main
Mar 21, 2025
Merged

Implemented chcpu#234
sylvestre merged 2 commits intouutils:mainfrom
mdcssw:main

Conversation

@koutheir
Copy link
Copy Markdown
Contributor

@koutheir koutheir commented Feb 24, 2025

Closes #22.

@koutheir koutheir changed the title Configure command line parsing Configure command line parsing for chcpu Feb 24, 2025
@koutheir koutheir changed the title Configure command line parsing for chcpu Implemented chcpu Feb 25, 2025
@koutheir
Copy link
Copy Markdown
Contributor Author

@sylvestre, this is util-linux. Why is the CI server attempting to build it for Windows and MacOS?

@koutheir
Copy link
Copy Markdown
Contributor Author

I just noticed PR #226. I don't intend to trigger any conflicts. @sylvestre, how should I proceed?

@sylvestre
Copy link
Copy Markdown
Contributor

@koutheir could you please contribute to the other PR ?
for example, it doesn't have test
wdyt?

@koutheir
Copy link
Copy Markdown
Contributor Author

koutheir commented Mar 8, 2025

could you please contribute to the other PR ?
for example, it doesn't have test

Does that mean abandoning the code of this pull request?

@sylvestre
Copy link
Copy Markdown
Contributor

i haven't looked which one is the best implementation
maybe we can merge the two?

@koutheir
Copy link
Copy Markdown
Contributor Author

koutheir commented Mar 8, 2025

I actually read the code of the other pull request, and given the significant difference in design decisions, I don't think the two PRs can be merged, without overriding most of the code of either implementations.

@sylvestre
Copy link
Copy Markdown
Contributor

OK. Too bad.
I didn't look at the feature scope but in general the first wins.

@sylvestre
Copy link
Copy Markdown
Contributor

I will have a deeper look

@koutheir
Copy link
Copy Markdown
Contributor Author

@sylvestre, did you get a chance to look deeper?

@sylvestre
Copy link
Copy Markdown
Contributor

not yet, sorry

@sylvestre
Copy link
Copy Markdown
Contributor

could you please fix the build failure on other OS ?
I'm prefer this implementation because it offers superior modularity with better organized files and robust error handling.

@koutheir
Copy link
Copy Markdown
Contributor Author

could you please fix the build failure on other OS ?

Should I remove the Windows and MacOS entries from the CI configuration?

@sylvestre
Copy link
Copy Markdown
Contributor

nope, fix your PR to make it pass on these archs (as much as possible)

it was green before: https://github.com/uutils/util-linux/actions/runs/13933228399

@koutheir
Copy link
Copy Markdown
Contributor Author

fix your PR to make it pass on these archs (as much as possible)

Do you mean port the tools to these platforms? Or somehow exclude the tool from building? Or make it an empty crate?

@sylvestre
Copy link
Copy Markdown
Contributor

As much as possible, we want to support other OS. Here, i don't know if there are API to do the same on Windows and Mac

@koutheir
Copy link
Copy Markdown
Contributor Author

For reference, these tools (util-linux tools) are very Linux-specific, relying on Linux-only APIs and mechanisms. Porting them even to other UNIXes is tricky. Porting them to Windows too will make the tools far more complicated, and probably less useful than native tools already offered by these platforms.

In the meantime, I'll try to force the tool to "compile" on platforms other than Linux, but I'll mark things as unimplemented, so if anybody tries to actually use them, they would get a reasonable error.

@sylvestre
Copy link
Copy Markdown
Contributor

sounds good, thanks!

@koutheir
Copy link
Copy Markdown
Contributor Author

@sylvestre, I applied all your requests.

@sylvestre sylvestre merged commit f8e425b into uutils:main Mar 21, 2025
13 checks passed
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (f91b55d) to head (31795f2).
Report is 8 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #234   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sylvestre sylvestre mentioned this pull request Mar 21, 2025
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.

Implement program chcpu

2 participants