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

use explicit dependencies in src/app #10455

Merged
merged 12 commits into from
Mar 18, 2022

Conversation

ylecornec
Copy link
Collaborator

@ylecornec ylecornec commented Mar 10, 2022

This PR sets (implicit_transitive_deps false) in the dune-project files of src/app,
adds the necessary explicit dependencies and removes some unnecessary ones.

Explain how you tested your changes:

  • dune build in the affected folders
  • The CI failure seems unrelated
  • make build
    make build_all_sigs
    make build_archive
    make build_archive_all_sigs
    make build_rosetta
    make build_intgtest
    

Checklist:

  • Modified the current draft of release notes with details on what is completed or incomplete within this project
  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them

@ylecornec ylecornec added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Mar 10, 2022
@ylecornec ylecornec changed the title Ylecornec/explicit dependencies src app use explicit dependencies in src/app Mar 10, 2022
@ylecornec ylecornec marked this pull request as ready for review March 10, 2022 15:27
@ylecornec ylecornec requested review from a team as code owners March 10, 2022 15:27
Copy link
Contributor

@lk86 lk86 left a comment

Choose a reason for hiding this comment

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

LGTM but you will have to resolve the conflicts and get CI to pass, whether the failure is related or not

@ylecornec ylecornec force-pushed the ylecornec/explicit_dependencies_src_app branch from 9eb96e2 to 0cf8cbb Compare March 18, 2022 12:36
Conflicts:
	src/app/archive/archive_lib/dune
	src/app/cli/src/init/dune
@ylecornec ylecornec force-pushed the ylecornec/explicit_dependencies_src_app branch from 0cf8cbb to b599da9 Compare March 18, 2022 15:18
@ylecornec ylecornec merged commit 7de9bd7 into develop Mar 18, 2022
@ylecornec ylecornec deleted the ylecornec/explicit_dependencies_src_app branch March 18, 2022 17:31
@robinbb robinbb added the Tweag label Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants