Skip to content

Add osname()#27

Merged
sylvestre merged 11 commits intouutils:mainfrom
rivy:add.osname
Jan 8, 2023
Merged

Add osname()#27
sylvestre merged 11 commits intouutils:mainfrom
rivy:add.osname

Conversation

@rivy
Copy link
Contributor

@rivy rivy commented Jan 7, 2023

This is on top of PR #26 (see the changes; I'll rebase with #26 is merged).

Move osname() calculation down from uutils uname into platform-info to reduce uname complexity and add additional WinOS info to the result.

With this and updates to uname, a future uname -a would look like...

> uname -a
Windows_NT HOSTNAME 10.0 19044 x86_64 MS/Windows (Windows 10)

@rivy rivy mentioned this pull request Jan 7, 2023
@rivy
Copy link
Contributor Author

rivy commented Jan 7, 2023

Is it time to remove "AppVeyor" CI?

@sylvestre
Copy link
Contributor

Is it time to remove "AppVeyor" CI?

Yeah

println!("release = {}", uname.release());
println!("version = {}", uname.version());
println!("machine = {}", uname.machine());
println!("osname = {}", uname.osname());
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could add per platform tests as in this case, we know what we will get, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look at it tonight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the redundant WinOS test, and added platform-specific tests of osname() for 'unix' and 'windows'.

@rivy
Copy link
Contributor Author

rivy commented Jan 8, 2023

Updated tests and rebased onto 'main'.

@rivy
Copy link
Contributor Author

rivy commented Jan 8, 2023

I added two new WinOS tests: 1) compare version from DLL vs version from file against each other, and 2) confirm known version/os_name combinations.

It looks like coverage improves from about 45% to 75%+.

@sylvestre sylvestre merged commit b315036 into uutils:main Jan 8, 2023
@sylvestre
Copy link
Contributor

Is it time to remove "AppVeyor" CI?

Done

@sylvestre
Copy link
Contributor

sylvestre commented Jan 8, 2023

@rivy woud you like a new release with your changes?

@rivy
Copy link
Contributor Author

rivy commented Jan 8, 2023

@rivy woud you like a new release with your changes?

Yes, please.

I just noticed the CodeCov badge wasn't updating... I just posted PR #30 fixing that issue.

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.

2 participants

Comments