-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
some refactorings #17
Conversation
…amily as input note that previously os_family and os_distribution were used: "bsd", "freebsd" -> pkg "debian", _ -> apt and now, os and os_family are used: "freebsd", _ -> pkg "linux", "debian" -> apt
| _ -> None | ||
in | ||
OpamFilter.filter_formula ~default:false env depends |
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.
the reason to remove this ~default:false
is that we notice future missing variables. we can re-add it if it is annoying.
the code is called from Orb with a try .. catch
anyways, thus the filter_formula
raising an exception is handled.
"OS_DISTRIBUTION", vars.os_distribution; | ||
"OS_VERSION", vars.os_version; | ||
"OS_FAMILY", vars.os_family; | ||
"ARCH", vars.arch; |
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.
we now include arch as well.
log "unsupported OS (no system packages), bsd without distribution" | ||
end | ||
| Some "debian" -> | ||
let dump_system_packages ~os ~os_family filename = |
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.
as mentioned in the commit message, using os and os_family looks sufficient (for now).
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.
This looks nice to me
this also fixes #16