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

lib: refactor actually a lot of stuff (sorry mic) #294

Merged
merged 14 commits into from
Jul 19, 2023
Merged

lib: refactor actually a lot of stuff (sorry mic) #294

merged 14 commits into from
Jul 19, 2023

Conversation

Lassulus
Copy link
Collaborator

No description provided.

@Lassulus Lassulus force-pushed the refactor3 branch 11 times, most recently from 4de3fa5 to 04e7af6 Compare July 15, 2023 15:12
@Lassulus Lassulus changed the title lib: refactor outputs into toplevel lib: refactor actually a lot of stuff Jul 15, 2023
@Lassulus Lassulus changed the title lib: refactor actually a lot of stuff lib: refactor actually a lot of stuff (sorry mic) Jul 15, 2023
@Lassulus
Copy link
Collaborator Author

Lassulus commented Jul 15, 2023

not sure if the cli is still working, since we have no tests for it :(

EDIT: I remembered how to use the cli and it is broken :((

EDIT2: yay I fixed it again :)

@@ -85,7 +85,7 @@ A simple disko configuration may look like this:
If you'd saved this configuration in /tmp/disko-config.nix, and wanted to create a disk named /dev/nvme0n1, you would run the following command to partition, format and mount the disk.

```
$ sudo nix run github:nix-community/disko -- --mode zap_create_mount /tmp/disko-config.nix --arg disks '[ "/dev/nvme0n1" ]'
$ sudo nix run github:nix-community/disko -- --mode disko /tmp/disko-config.nix --arg disks '[ "/dev/nvme0n1" ]'
Copy link
Member

Choose a reason for hiding this comment

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

Do we have some documentation what each of these modes do?
It's not exactly intuitive from the name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added some documentation to the help output of the disko cli

mountScript = cfg: pkgs: ((eval cfg).config.disko.devices._scripts { inherit pkgs checked; }).mountScript;
mountScriptNoDeps = cfg: pkgs: ((eval cfg).config.disko.devices._scripts { inherit pkgs checked; }).mountScriptNoDeps;

disko = cfg: (eval cfg).config.disko.devices._disko;
Copy link
Member

@Mic92 Mic92 Jul 17, 2023

Choose a reason for hiding this comment

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

So disko is the new umount+format+mount?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

create is an alias for format. disko is umount + format + mount

Copy link
Member

Choose a reason for hiding this comment

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

As I just learned it's umount + destroy + format + mount

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

umount and destroy are the same thing right now

disko Show resolved Hide resolved
lib/default.nix Outdated Show resolved Hide resolved
lib/types/mdadm.nix Outdated Show resolved Hide resolved
Copy link
Member

@lilyinstarlight lilyinstarlight left a comment

Choose a reason for hiding this comment

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

Do we intend to continue supporting { type = "table"; format = "gpt"; ... } in addition to { type = "gpt"; ... }?

This removes that former construction from all tests so it is now untested on this branch from what I can tell

@Lassulus
Copy link
Collaborator Author

Do we intend to continue supporting { type = "table"; format = "gpt"; ... } in addition to { type = "gpt"; ... }?

This removes that former construction from all tests so it is now untested on this branch from what I can tell

not really. But I'm not sure if type = "gpt" breaks in some scenarios

@Lassulus
Copy link
Collaborator Author

added a legacy table test, renamed _umount to _destroy.

@Lassulus
Copy link
Collaborator Author

ah forget the test, I also included some other stale PRs which I took a look at

@Lassulus
Copy link
Collaborator Author

6068287 (#294) could be a breaking change for nixos-anywhere, hmm. not sure if we should include it here.

@Mic92
Copy link
Member

Mic92 commented Jul 19, 2023

6068287 (#294) could be a breaking change for nixos-anywhere, hmm. not sure if we should include it here.

I cannot find this commit.

@Lassulus
Copy link
Collaborator Author

4b9126c

@Lassulus
Copy link
Collaborator Author

Basically what #78 does

@Lassulus
Copy link
Collaborator Author

ok dropped the commit for now, we need better migrations for that one

@Mic92
Copy link
Member

Mic92 commented Jul 19, 2023

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Jul 19, 2023

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 774ce7d

@mergify mergify bot merged commit 774ce7d into master Jul 19, 2023
31 checks passed
@mergify mergify bot deleted the refactor3 branch July 19, 2023 19:07
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.

None yet

4 participants