Skip to content

Make -strict-formats the default, but have -no-strict-formats to disable it#514

Closed
DemiMarie wants to merge 6 commits intoocaml:trunkfrom
DemiMarie:strict-by-default-really
Closed

Make -strict-formats the default, but have -no-strict-formats to disable it#514
DemiMarie wants to merge 6 commits intoocaml:trunkfrom
DemiMarie:strict-by-default-really

Conversation

@DemiMarie
Copy link
Copy Markdown
Contributor

This makes -strict-formats the default. It can be disabled with the (deprecated) -no-strict-formats.

@gasche
Copy link
Copy Markdown
Member

gasche commented Mar 17, 2016

Unfortunately it is unreasonable to merge both options in the same release, because that means that people will have no backward-compatible way to disable the feature. I'm sorry for this glaring omission of the format work. We should merge -no-strict-formats as soon as possible, and wait a bit before considering a switch in default.

Thanks a lof for the PR though; this is a glaring omission. Would you be ready to rebase the PR with just support for -no-strict-formats and no default change?

@damiendoligez would you agree with a -no-strict-formats option merged in 4.03?

(We should think of adding -no-strict-sequence as well, just in case.)

@DemiMarie
Copy link
Copy Markdown
Contributor Author

@gasche Should I issue that PR against 4.03? If so, it might be possible to add -no-strict-formats in 4.03 and make -strict-formats the default in 4.04?

@gasche
Copy link
Copy Markdown
Member

gasche commented Mar 17, 2016

Don't both with the 4.03/4.04 thing, I can change it at merge time.

We can make -strict-formats the default as soon as all OCaml versions the majority of our users care about support -no-strict-formats. (My personal test is to look at what Debian Stable has, but there is no hard rule.) I think we aim for a shorter release cycle for 4.04, so it may actually be released too fast for that.

@alainfrisch
Copy link
Copy Markdown
Contributor

because that means that people will have no backward-compatible way to disable the feature

I generally agree with this line of thoughts ( and this is actually why I opened http://caml.inria.fr/mantis/view.php?id=7184 ). But for this specific case, fixing invalid formats should be rather straightforward and would give an easy way to make the code base compatible with whatever release.

@gasche
Copy link
Copy Markdown
Member

gasche commented Mar 17, 2016

I think this is a good argument, thanks. Thinking about it in this way, I would be ready to always enable strict formats (making the configuration option a legacy, ignored option) in trunk (4.04+dev).

(-no-strict-sequence is still an important flag that it would make sense to have as soon as possible, including in 4.03, I think.)

@damiendoligez
Copy link
Copy Markdown
Member

(-no-strict-sequence is still an important flag that it would make sense to have as soon as possible, including in 4.03, I think.)

Agreed. Here is a list of other flags to think about:

-no-keep-docs
-no-keep-locs
-alias-deps
-app-funct
-no-principal
-no-rectypes

@damiendoligez damiendoligez added this to the 4.03.0 milestone Mar 18, 2016
@gasche
Copy link
Copy Markdown
Member

gasche commented Mar 18, 2016

We can have all of them for the pleasure of completeness, but I don't see OCaml switching to ambiant recursive types by the time 4.03 hits Debian stable. All the others definitely make sense. Do I understand correctly that you ( @damiendoligez ) would be interested in a patch implementing all these (trivial) flags? What is your inclination for (-no)-strict-formats?

@damiendoligez
Copy link
Copy Markdown
Member

Implement them all (including -no-strict-formats), we'll switch the default to -strict-formats in 4.04.

@damiendoligez
Copy link
Copy Markdown
Member

@drbo
Are you willing to change this PR to add the missing flags and remove the switch to strict-formats?
I would need this within a few days.

@DemiMarie DemiMarie force-pushed the strict-by-default-really branch from 6d21909 to 7e8c001 Compare April 6, 2016 05:18
@DemiMarie
Copy link
Copy Markdown
Contributor Author

@damiendoligez Fixed and pushed

Tools:
======

- GPR#514: Added several flags:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"compiler flags" would be more explicit. You may want to mention that we should always have both the -foo and -no-foo options, so that people can always explicitly express their intent and be robust if the default setting changes.

@gasche
Copy link
Copy Markdown
Member

gasche commented Apr 6, 2016

I reviewed the proposed patch and it looked fine.

@gasche
Copy link
Copy Markdown
Member

gasche commented Apr 18, 2016

I'm about to merge the branch in 4.03 and trunk.

@drbo it appears to me that, despite what the Changes entry says, you do not support -app-funct and and -alias-deps.

@gasche
Copy link
Copy Markdown
Member

gasche commented Apr 18, 2016

I rebased and merged in trunk ( 795a4d5 ) and 4.03 ( 0f4fea9 ). Thanks!

Feel free to ping us after the release if we forget to make -strict-formats the default in 4.04+dev. (I'd rather not do it right now to avoid diverting effort handling any format-breakage from the release work.)

@gasche gasche closed this Apr 18, 2016
DemiMarie added a commit to DemiMarie/ocaml that referenced this pull request Apr 19, 2016
This was meant for GPR ocaml#514, but I forgot to include it.
gasche pushed a commit that referenced this pull request Apr 19, 2016
This was meant for GPR #514, but I forgot to include it.
gasche pushed a commit that referenced this pull request Apr 19, 2016
This was meant for GPR #514, but I forgot to include it.
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jun 30, 2021
…#514)

PR ocaml#470 propagates substitutions down into lambdas, which works when the
RHS is a constant or symbol but not if it's a variable, since that
variable is now out of scope. This was breaking ocaml#485, which produces
variable-for-variable substitutions often. Fortunately,
variable-for-variable substitution under a lambda is also unnecessary
(we already dealt with the free occurrences by making a closure element),
so we can happily just filter out any such bindings.
EduardoRFS pushed a commit to esy-ocaml/ocaml that referenced this pull request Jul 23, 2021
Update instructions in ocaml-variants.opam
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
The middleware was only applied when a match was found for the route in the defined scope, which defeats the purpose of handling unknown routes ending with a slash to the correct routes.

This commit makes the trailing slash middleware global to every request.
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.

4 participants