MatthewKing / DeviceId

A simple library providing functionality to generate a 'device ID' that can be used to uniquely identify a computer.
MIT License
794 stars 118 forks source link

Mistake in migration guide? #55

Closed derekantrican closed 2 years ago

derekantrican commented 2 years ago

The migration guide in the readme states that for AddOSInstallationID (on mac) the equivalent in v6+ should be AddSystemDriveSerialNumber but - looking at the v5 & v6 implementations I think the README should state the equivalent is actually AddPlatformSerialNumber.

AddOSInstallationID v5:

https://github.com/MatthewKing/DeviceId/blob/fd2fb79be80cbb3130d3df94e2ceed4d03514132/src/DeviceId/DeviceIdBuilderExtensions.cs#L186-L189

AddPlatformSerialNumber v6:

https://github.com/MatthewKing/DeviceId/blob/814e752699071e756cf0b1db55252367cd60675b/src/DeviceId.Mac/MacDeviceIdBuilderExtensions.cs#L28

The commands executed are identical between these two (and not in AddSystemDriveSerialNumber)

derekantrican commented 2 years ago

Also, did the default formatter change in v6+?

If I do the following in v5 and v6:

Using the ExampleCustomFormatter from here to print out the raw values.

Console.WriteLine(new DeviceIdBuilder().AddOSInstallationID().UseFormatter(new ExampleCustomFormatter()));
Console.WriteLine(new DeviceIdBuilder().AddSystemDriveSerialNumber().UseFormatter(new ExampleCustomFormatter()));
Console.WriteLine(new DeviceIdBuilder().AddOSInstallationID().AddSystemDriveSerialNumber());

I get essentially (values simplified because I don't want to find and copy down the actual values):

v5:

v6:

So the raw value is getting retrieved the same between v5 & v6 but the default formatter must have changed. If this is the case, I would suggest adding it to the migration guide (as I don't see anything about it on there).

MatthewKing commented 2 years ago

Thanks, I'll need to have a look at the migration guide to double-check everything.

The default formatter did change in V6. This is mentioned in the formatting section but not in the migration guide. I'll add it to the migration guide as well when I get some time.

MatthewKing commented 2 years ago

Think I've fixed up the migration guide now, too. Better late than never, I guess.