-
Notifications
You must be signed in to change notification settings - Fork 27
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
Improve demote - move broken packages to the end #395
Conversation
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.
Seems like a reasonable improvement to me, just a small nitpick from my side.
lib/opam_solve.ml
Outdated
List.partition versions ~f:(fun (_version, opam_res) -> | ||
let avoid_versions, broken_versions, regular_versions = | ||
List.fold_left versions ~init:([], [], []) | ||
~f:(fun (avoid, broken, regular) ((_, opam_res) as e) -> |
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.
Could you reorder this to be regular, avoid, broken
to match the final way they get appended?
Updated with the suggestion |
Perfect, thanks! |
CHANGES: ### Added - Display warning when a package to be locked is missing a `dev-repo` field and is being skipped because of it (tarides/opam-monorepo#341, tarides/opam-monorepo#362, @kit-ty-kate, @Leonidas-from-XIV) - Add option `--keep-symlinked-dir` to preserve symlinks in `duniverse/`, which can be useful for local development. (tarides/opam-monorepo#348, tarides/opam-monorepo#366, @hannesm, @Leonidas-from-XIV) - Adopt the OCaml Code of Conduct (tarides/opam-monorepo#391, @rikusilvola) - Add solver tests (tarides/opam-monorepo#394, @samoht) ### Changed - Canonicalize the URLs of the OPAM `dev-repo` fields to be able to detect more semantically equivalent URLs, this should reduce the risk of build failures due to duplicate code pulled (tarides/opam-monorepo#118, tarides/opam-monorepo#365 @TheLortex, @Leonidas-from-XIV) - Simple the error message printed when dependencies don't use dune as their build system. The opam-0install diagnostic message is no longer printed in this case and the message has been reformatted and reworded to make the salient information easier to see. (tarides/opam-monorepo#384, @gridbugs) - Encode `dev-repo` constraints in the opam solver - this allows to resolve more involved version constraints that were failing before (tarides/opam-monorepo#396, @samoht) ### Deprecated ### Fixed - Error in case where multiple packages with different dev-repo fields would be placed in the same duniverse directory (tarides/opam-monorepo#377, @gridbugs) - Fix a failure when using opam-monorepo with an opam 2.2 root (tarides/opam-monorepo#379, @kit-ty-kate) - Fix assertion failure when prefix of "lock" subcommand is used (tarides/opam-monorepo#381, @gridbugs) - Treat packages without build commands as virtual only if also lack install commands, as some non-virtual packages might only have install commands. (tarides/opam-monorepo#376 @Leonidas-from-XIV, @gridbugs) - Improve the ordering of package candidates by putting broken packages at then end of the list (tarides/opam-monorepo#395, tarides/opam-monorepo#397, @samoht) ### Removed ### Security
CHANGES: ### Added - Display warning when a package to be locked is missing a `dev-repo` field and is being skipped because of it (tarides/opam-monorepo#341, tarides/opam-monorepo#362, @kit-ty-kate, @Leonidas-from-XIV) - Add option `--keep-symlinked-dir` to preserve symlinks in `duniverse/`, which can be useful for local development. (tarides/opam-monorepo#348, tarides/opam-monorepo#366, @hannesm, @Leonidas-from-XIV) - Adopt the OCaml Code of Conduct (tarides/opam-monorepo#391, @rikusilvola) - Add solver tests (tarides/opam-monorepo#394, @samoht) ### Changed - Canonicalize the URLs of the OPAM `dev-repo` fields to be able to detect more semantically equivalent URLs, this should reduce the risk of build failures due to duplicate code pulled (tarides/opam-monorepo#118, tarides/opam-monorepo#365 @TheLortex, @Leonidas-from-XIV) - Simple the error message printed when dependencies don't use dune as their build system. The opam-0install diagnostic message is no longer printed in this case and the message has been reformatted and reworded to make the salient information easier to see. (tarides/opam-monorepo#384, @gridbugs) - Encode `dev-repo` constraints in the opam solver - this allows to resolve more involved version constraints that were failing before (tarides/opam-monorepo#396, @samoht) ### Deprecated ### Fixed - Error in case where multiple packages with different dev-repo fields would be placed in the same duniverse directory (tarides/opam-monorepo#377, @gridbugs) - Fix a failure when using opam-monorepo with an opam 2.2 root (tarides/opam-monorepo#379, @kit-ty-kate) - Fix assertion failure when prefix of "lock" subcommand is used (tarides/opam-monorepo#381, @gridbugs) - Treat packages without build commands as virtual only if also lack install commands, as some non-virtual packages might only have install commands. (tarides/opam-monorepo#376 @Leonidas-from-XIV, @gridbugs) - Improve the ordering of package candidates by putting broken packages at then end of the list (tarides/opam-monorepo#395, tarides/opam-monorepo#397, @samoht) ### Removed ### Security
CHANGES: ### Added - Display warning when a package to be locked is missing a `dev-repo` field and is being skipped because of it (tarides/opam-monorepo#341, tarides/opam-monorepo#362, @kit-ty-kate, @Leonidas-from-XIV) - Add option `--keep-symlinked-dir` to preserve symlinks in `duniverse/`, which can be useful for local development. (tarides/opam-monorepo#348, tarides/opam-monorepo#366, @hannesm, @Leonidas-from-XIV) - Adopt the OCaml Code of Conduct (tarides/opam-monorepo#391, @rikusilvola) - Add solver tests (tarides/opam-monorepo#394, @samoht) ### Changed - Canonicalize the URLs of the OPAM `dev-repo` fields to be able to detect more semantically equivalent URLs, this should reduce the risk of build failures due to duplicate code pulled (tarides/opam-monorepo#118, tarides/opam-monorepo#365 @TheLortex, @Leonidas-from-XIV) - Simple the error message printed when dependencies don't use dune as their build system. The opam-0install diagnostic message is no longer printed in this case and the message has been reformatted and reworded to make the salient information easier to see. (tarides/opam-monorepo#384, @gridbugs) - Encode `dev-repo` constraints in the opam solver - this allows to resolve more involved version constraints that were failing before (tarides/opam-monorepo#396, @samoht) ### Deprecated ### Fixed - Error in case where multiple packages with different dev-repo fields would be placed in the same duniverse directory (tarides/opam-monorepo#377, @gridbugs) - Fix a failure when using opam-monorepo with an opam 2.2 root (tarides/opam-monorepo#379, @kit-ty-kate) - Fix assertion failure when prefix of "lock" subcommand is used (tarides/opam-monorepo#381, @gridbugs) - Treat packages without build commands as virtual only if also lack install commands, as some non-virtual packages might only have install commands. (tarides/opam-monorepo#376 @Leonidas-from-XIV, @gridbugs) - Improve the ordering of package candidates by putting broken packages at then end of the list (tarides/opam-monorepo#395, tarides/opam-monorepo#397, @samoht) ### Removed ### Security
No description provided.