-
Notifications
You must be signed in to change notification settings - Fork 105
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
Feature/require script consistency #1451
Conversation
a7e768e
to
93033e4
Compare
wip: Getting async js feat: Have execute get action config feat: Read + Write chore: Add typing for globals chore: Change the default path, include error on missing function, and add json File Read Write chore: Change the default path, include error on missing function, and add json File Read Write wip: Fix the unit test wip: Fix the unit test feat: module loading
93033e4
to
238c1c6
Compare
@@ -8,7 +8,7 @@ if [ "$0" != "./build-prod.sh" ]; then | |||
exit 1 | |||
fi | |||
|
|||
alias 'rust-arm64-builder'='docker run --rm -it -v "$HOME/.cargo/registry":/root/.cargo/registry -v "$(pwd)":/home/rust/src start9/rust-arm-cross:aarch64' | |||
alias 'rust-arm64-builder'='docker run --rm -it -v "$HOME/.cargo/registry":/root/.cargo/registry -v "$(pwd)":/home/rust/src -P start9/rust-arm-cross:aarch64' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't I remove this?
@@ -154,6 +154,29 @@ pub struct Manifest { | |||
pub dependencies: Dependencies, | |||
} | |||
|
|||
impl Manifest { | |||
pub fn package_procedures(&self) -> impl Iterator<Item = &PackageProcedure> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 I don't like the looks of this but there are no working solutions. I think that a generator syntax would work better.
None | ||
let needs_script = manifest.package_procedures().any(|a| a.is_script()); | ||
let has_script = script_path.exists(); | ||
match (needs_script, has_script) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 Neat. I kinda like this pattern, but if we got more than two, maybe having enums to try and prevent boolean blindness?
match (needs_script, has_script) {
(AreScripts, HasScripts) => ...,
(AreScript, MissingScripts) => ...,
(NoScripts, HasScripts) => ...,
(NoScripts, MissingScripts) => None,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it
after #1437