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

Distinst Drive Detection #263

Merged
merged 6 commits into from
Mar 20, 2018
Merged

Conversation

mmstick
Copy link
Collaborator

@mmstick mmstick commented Mar 19, 2018

Closes #180, Closes #209, Closes #230, Closes #231

@cassidyjames
Copy link
Contributor

With this change I'm getting a much more verbose and seemingly machine-friendly string for the drive names, instead of the more human-friendly names like before.

@cassidyjames
Copy link
Contributor

Before:
image

After:
image

@mmstick
Copy link
Collaborator Author

mmstick commented Mar 19, 2018

@cassidyjames How's this?

screenshot from 2018-03-19 16-51-51

jackpot51
jackpot51 previously approved these changes Mar 19, 2018
@mmstick
Copy link
Collaborator Author

mmstick commented Mar 19, 2018

screenshot from 2018-03-19 18-06-33

Now checking for disk sizes

next_button.sensitive = false;
}
});
if (size < 5000000000) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be a configuration option, or a value read from the /cdrom/casper/filesystem.size file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jackpot51 Think I should implement that logic in distinst?

jackpot51
jackpot51 previously approved these changes Mar 20, 2018
jackpot51
jackpot51 previously approved these changes Mar 20, 2018
@cassidyjames
Copy link
Contributor

Is /sdb and /sdc really useful to people in this context? At this point, the user is not on their familiar/installed system, so these paths might as well be meaningless. I would prefer we didn't diverge from the initial design without discussion and good reason; we've considered a lot of these things when we spent the time actually designing it.

@cassidyjames
Copy link
Contributor

Invalid drives should be sorted to the end, as covered in the Wiki.

@jackpot51
Copy link
Collaborator

@cassidyjames I agree with sorting the invalids at the end.

The /dev/sdb is useful for cross-referencing the drive in other tools like GParted.

@jackpot51
Copy link
Collaborator

jackpot51 commented Mar 20, 2018

Especially if you have two identical drives as is common in many systems. For example, my desktop at home has two Samsung EVO 850 SSDs. Seing simply "Samsung EVO 850" twice does not give me enough information to decide which drive to install to.

Perhaps when we have OS detection, this will be easier to determine with an OS icon and/or name, but for now I think it is very useful to be able to see the path that other tools in the live disk will display when referring to the drive.

@cassidyjames
Copy link
Contributor

cassidyjames commented Mar 20, 2018

@jackpot51 the custom partition view is a separate concern, as that will be an entirely different view with different information. If you're using this screen, it's not likely that you're going to know or care what the path to the drive is. And if the intent is to distinguish two identical drives, we could be smart about actually solving that by only showing it when the drives are in fact the same.

Is the path likely to be the same across OSes/boots? If so, I guess I'm fine with showing it, but it needs to be secondary information instead of on the same line as the drive name. I'd recommend putting it on the second line before the size.

- Move path to underneath name
@jackpot51
Copy link
Collaborator

installer-sort

I addressed those two items - moving the path to the next line and putting disabled disks at the end.

jackpot51
jackpot51 previously approved these changes Mar 20, 2018
@jackpot51 jackpot51 merged commit b3ec619 into elementary:master Mar 20, 2018
@isantop
Copy link
Collaborator

isantop commented Mar 20, 2018

I do agree with @jackpot51 here. Displaying two completely identical entries with no way to distinguish them seems like a bad experience, especially when one has my important daily-driver OS that needs to be stable, while the other one is my willy-nilly test drive for whatever I feel like running. Even if the drive paths are different from my normal OS (They're very likely to be the same among all Ubuntu-based OSs from the last decade or so), displaying the /dev path allows me to use other tools within the system if necessary to separate them and install the OS to the correct drive, preventing me from losing important data.

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.

4 participants