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

Stubbed out all builtin functions/tests #121

Merged
merged 4 commits into from
May 6, 2024
Merged

Stubbed out all builtin functions/tests #121

merged 4 commits into from
May 6, 2024

Conversation

arduano
Copy link
Collaborator

@arduano arduano commented Apr 27, 2024

Added stubs for all builtins and their respective test modules, ordered in the order they appear in the Nix manual
https://nixos.org/manual/nix/stable/language/builtins.html

@@ -256,6 +244,199 @@ mod attr_values {
}
}

mod baseNameOf {
use super::*;
Copy link
Owner

Choose a reason for hiding this comment

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

Mind adding a couple of tests for this function? Feel free to do in a separate PR.

urbas
urbas previously approved these changes Apr 29, 2024
@arduano
Copy link
Collaborator Author

arduano commented Apr 29, 2024

@urbas Can you please approve again, or enable approvals to stay after commits

@arduano arduano requested a review from urbas April 29, 2024 22:04
@arduano
Copy link
Collaborator Author

arduano commented Apr 30, 2024

Also FYI

For the next PR I'm planning to try using graphite https://graphite.dev/
There will be multiple PRs referencing each other, they all depend on each other but it makes it easier for me to divide logical changes from 1 massive PR to like 5+ smaller PRs for you to review independently

I've used graphite before, but would be interesting to try it on a smaller project like this

);
assert_eq!(
eval_ok("builtins.baseNameOf \"/foo/bar/baz/\""),
Value::Str("".into())
Copy link
Owner

@urbas urbas May 6, 2024

Choose a reason for hiding this comment

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

Nix seems to return "baz" for this test:

% nix eval --expr 'builtins.baseNameOf "/foo/bar/baz/"'
"baz"
Suggested change
Value::Str("".into())
Value::Str("baz".into())

Btw, the basename CLI tool also returns baz in this case.

@urbas
Copy link
Owner

urbas commented May 6, 2024

Also FYI

For the next PR I'm planning to try using graphite https://graphite.dev/ There will be multiple PRs referencing each other, they all depend on each other but it makes it easier for me to divide logical changes from 1 massive PR to like 5+ smaller PRs for you to review independently

I've used graphite before, but would be interesting to try it on a smaller project like this

I haven't used graphite before, will give it a go.

Btw, apologies for the slow responses. Was traveling a lot these days.

@urbas urbas merged commit fa867b4 into master May 6, 2024
2 checks passed
@urbas urbas deleted the stub-builtins branch May 6, 2024 11:59
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