Skip to content

migrate scion default#715

Merged
eljamm merged 27 commits into
ngi-nix:mainfrom
khabdrick:migrate-scion
Apr 10, 2025
Merged

migrate scion default#715
eljamm merged 27 commits into
ngi-nix:mainfrom
khabdrick:migrate-scion

Conversation

@khabdrick
Copy link
Copy Markdown
Contributor

@khabdrick khabdrick commented Apr 4, 2025

Migrate scion default.nix to new project folder.

fixed #590

Comment thread projects/SCION/default.nix Outdated
Comment on lines +7 to +10
# NOTE:
# - Check `projects/models.nix` for a more detailed project structure
# - Each program/service must have at least one example
# - Set attributes to `null` to indicate that they're needed, but not available
Copy link
Copy Markdown
Contributor

@eljamm eljamm Apr 4, 2025

Choose a reason for hiding this comment

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

This is just a not for guidance. Please remove it, @khabdrick

Suggested change
# NOTE:
# - Check `projects/models.nix` for a more detailed project structure
# - Each program/service must have at least one example
# - Set attributes to `null` to indicate that they're needed, but not available

Comment thread projects/SCION/default.nix Outdated
Comment on lines +14 to +15
"FooBar"
"FooBar-cli"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Search for the project in the NLnet projects page and include the URL handle for the things related to SCION (see triaging template for an example).

Comment thread projects/SCION/default.nix Outdated
foobar-cli = null;
};

# NOTE: same structure as programs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This note means that the structure of nixos.modules.program above needs to be copied here, if the project uses services.

@eljamm
Copy link
Copy Markdown
Contributor

eljamm commented Apr 7, 2025

@khabdrick The project template has been updated. Is it more clear now?

@khabdrick
Copy link
Copy Markdown
Contributor Author

@eljamm Heeey!
I'm sorry. I have been ill for the past few days. I am looking at all your input now.

@khabdrick
Copy link
Copy Markdown
Contributor Author

Made some updates @eljamm

Copy link
Copy Markdown
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments @khabdrick. Hope you're doing better, now.

Comment thread projects/SCION/default.nix Outdated
Comment thread projects/SCION/default.nix Outdated
Comment on lines +24 to +27
# examples.foobar = {
# module = ./example.nix;
# description = "";
# tests.basic = ./test.nix;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can add some basic example and test, here

Comment thread projects/SCION/default.nix Outdated
Comment thread projects/SCION/default.nix Outdated
Comment thread projects/SCION/default.nix Outdated
Comment thread projects/SCION/default.nix Outdated
Comment on lines +51 to +64
links = {
build = {
text = "SCION Documentation";
url = "https://docs.scion.org/en/latest/";
};
build = {
text = "Build from source";
url = "https://github.com/scionproto/scion?tab=readme-ov-file#build-from-sources";
};
tests = {
text = "Testing Tutorial";
url = "https://docs.scion.org/en/latest/tutorials/deploy.html#tasks-to-perform";
};
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this might be redundant since they're the same links as nixos.modules.programs. We could just move them to the top-level metadata.links

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When I move them, I can delete from both nixos.modules.programs and nixos.modules.services?
@eljamm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yup

Comment thread projects/SCION/default.nix
@khabdrick
Copy link
Copy Markdown
Contributor Author

Thanks for addressing my comments @khabdrick. Hope you're doing better, now.

Yes I am, thank you.

khabdrick and others added 7 commits April 7, 2025 16:48
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
@khabdrick
Copy link
Copy Markdown
Contributor Author

I have made the updates.

Comment thread projects/SCION/programs/basic/examples/basic.nix Outdated
Comment thread projects/SCION/programs/basic/module.nix Outdated
Comment thread projects/SCION/programs/basic/module.nix Outdated
Comment thread projects/SCION/programs/basic/tests/basic.nix Outdated
Comment thread projects/SCION/programs/basic/tests/basic.nix Outdated
Comment thread projects/SCION/default.nix Outdated
Comment thread projects/SCION/default.nix Outdated
khabdrick and others added 7 commits April 8, 2025 09:51
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
@khabdrick
Copy link
Copy Markdown
Contributor Author

@eljamm sorry about so many typos. Didn't even realize.
What else is left for this to be completed.

Copy link
Copy Markdown
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

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

What else is left for this to be completed.

The failing tests need to pass. That's why you need to build this locally first, as the task instructs:

$ nix build .#checks.x86_64-linux.projects/SCION/nixos/examples/basic

Also, please run nix fmt when you're done.

Comment thread projects/SCION/default.nix
Comment thread projects/SCION/programs/basic/examples/basic.nix Outdated
Comment thread projects/SCION/programs/basic/tests/basic.nix Outdated
khabdrick and others added 3 commits April 8, 2025 11:02
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Comment thread projects/SCION/programs/basic/tests/basic.nix Outdated
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
@eljamm
Copy link
Copy Markdown
Contributor

eljamm commented Apr 8, 2025

Please always format the code with nix fmt before you commit.

@khabdrick
Copy link
Copy Markdown
Contributor Author

image
Looks like it's okay now? @eljamm

Copy link
Copy Markdown
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

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

Nice! You're very very close to the finish line.

in
{
options.programs.scion = {
enable = lib.mkEnableOption "scion";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you follow the new template and add packages from projects-old/SCION-1M here? After you finish, delete that directory and commit your changes.

@khabdrick
Copy link
Copy Markdown
Contributor Author

Done

@khabdrick
Copy link
Copy Markdown
Contributor Author

It seems like these packages are not available.

Comment thread projects/SCION/programs/basic/module.nix Outdated
khabdrick and others added 2 commits April 10, 2025 08:11
Co-authored-by: Fedi Jamoussi <Fedi.Jamoussi@protonmail.ch>
Copy link
Copy Markdown
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

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

Looks good. Congrats @khabdrick!

@eljamm eljamm merged commit 51793ff into ngi-nix:main Apr 10, 2025
2 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Nix@NGI Apr 10, 2025
themadbit pushed a commit to themadbit/ngipkgs that referenced this pull request Apr 10, 2025
OluchiTheAnalyst pushed a commit to OluchiTheAnalyst/ngipkgs that referenced this pull request Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Migrate SCION to projects

2 participants