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

Make --dev the default #920

Merged
2 commits merged into from Jun 28, 2018
Merged

Make --dev the default #920

2 commits merged into from Jun 28, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jun 27, 2018

Following this discussion this PR changes the default build profile to be dev. The rationale being that most of the time we want to compile the things we work on in development mode.

The -p flags which must be used in opam file still implies --profile release.

Since this is technically a breaking changes, this behavior is only enabled for the dune binary. To do so this PR adds a module Which_program indicating which one of dune or jbuilder the current running program is. It uses the workaround for library variants described here.

/cc @yminsky and @jberdine who tested the build profile feature

@ghost ghost requested review from rgrinberg and emillon June 27, 2018 18:15
doc/usage.rst Outdated
@@ -364,7 +364,7 @@ write a ``(profile ...)`` stanza. For instance:

.. code:: scheme

(profile dev)
(profile performances)
Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case, I believe that it is an is uncountable noun, and so we should use performance. Or maybe we can use a predefined profile name, like (profile release)?

Copy link
Author

Choose a reason for hiding this comment

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

ok, yes let's use release

$ dune runtest --root explicit-interfaces
Entering directory 'explicit-interfaces'
main alias runtest
hello

CR-someday diml: this is weird that we need to do that:
Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems weird, what happens if this is not added? If that's not fixable immediately, I think we should replace this CR- comment by a link to a bug report.

Copy link
Author

Choose a reason for hiding this comment

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

It seems that the file is not copied back from the source tree on every run. I've noticed this before but haven't take the time to investigate. I'll prepare a test to expose the behavior

Copy link
Author

Choose a reason for hiding this comment

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

I added a test in #923

Copy link
Author

Choose a reason for hiding this comment

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

For this PR I just changed the test to create the modified files in run.t

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that seems better that way indeed!

Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@ghost ghost merged commit 2982567 into ocaml:master Jun 28, 2018
This pull request was closed.
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