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

Allow . in c_names and cxx_names #2036

Merged
merged 2 commits into from Apr 9, 2019

Conversation

Projects
None yet
4 participants
@rgrinberg
Copy link
Member

commented Apr 9, 2019

Let's get a confirmation fro m @diml that this should be allowed. The fix itself is trivial.

Fix #2033

@rgrinberg rgrinberg requested review from diml and aalekseyev Apr 9, 2019

@rgrinberg rgrinberg changed the title Fix 2033 Fix #2033 Apr 9, 2019

@rgrinberg rgrinberg changed the title Fix #2033 Allow . in c_names and cxx_names Apr 9, 2019

@rgrinberg rgrinberg force-pushed the rgrinberg:fix-2033 branch from 8fe6a76 to dd93126 Apr 9, 2019

@aalekseyev
Copy link
Collaborator

left a comment

Any reason the test is negative (showing the error message) rather than positive (showing a successful compilation)?

Assuming the compilation succeeds when the file is added, this change seems obviously good to me.

@rgrinberg rgrinberg force-pushed the rgrinberg:fix-2033 branch from dd93126 to 42fc12f Apr 9, 2019

@rgrinberg rgrinberg requested review from ejgallego and emillon as code owners Apr 9, 2019

@rgrinberg

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

The reason is that I forgot to promote. Thanks for noticing.

@rgrinberg rgrinberg force-pushed the rgrinberg:fix-2033 branch from 42fc12f to cae017d Apr 9, 2019

@rgrinberg

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

Sorry @ejgallego I must have added you as a reviewer accidentally

@rgrinberg rgrinberg force-pushed the rgrinberg:fix-2033 branch 3 times, most recently from 2cb6a98 to 6bc722b Apr 9, 2019

@diml

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

I can't think of a reason why this shouldn't be allowed. I can't remember either why it wasn't allowed in the first place.

rgrinberg added some commits Apr 9, 2019

Add reproduction case for 2033
Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
Allow . in c_names and cxx_names
Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>

@rgrinberg rgrinberg force-pushed the rgrinberg:fix-2033 branch from 6bc722b to 6652739 Apr 9, 2019

@rgrinberg rgrinberg merged commit d300cde into ocaml:master Apr 9, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
DCO All commits are signed off!
Details

rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Apr 9, 2019

[new release] dune (1.9.0)
CHANGES:

- Warn when generated `.merlin` does not reflect the preprocessing
  specification. This occurs when multiple stanzas in the same directory use
  different preprocessing specifications. This warning can now be disabled with
  `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg)

- Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956,
  @emillon)

- Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg)

- Add support for library variants and default implementations. (ocaml/dune#1900,
  @TheLortex)

- Add experimental `$ dune init` command. This command is used to create or
  update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder)

- Experimental Coq support (fix ocaml/dune#1466, @ejgallego)

- Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix
  ocaml/dune#1973 @rgrinberg)

- Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997,
  @rgrinberg)

- Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008,
  @rgrinberg)

- Warn instead of failing if an opam file fails to parse. This opam file can
  still be used to define scope. (ocaml/dune#2023, @rgrinberg)

- Do not crash if unable to read a directory when traversing to find root
  (ocaml/dune#2024, @rgrinberg)

- Do not exit dune if some source directories are unreadable. Instead, warn the
  user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg)

- Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent
  `binaries` fields would be ignored, but instead they should be combined.
  (ocaml/dune#2029, @rgrinberg)

- Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg)

rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Apr 9, 2019

[new release] dune (1.9.0)
CHANGES:

- Warn when generated `.merlin` does not reflect the preprocessing
  specification. This occurs when multiple stanzas in the same directory use
  different preprocessing specifications. This warning can now be disabled with
  `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg)

- Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956,
  @emillon)

- Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg)

- Add support for library variants and default implementations. (ocaml/dune#1900,
  @TheLortex)

- Add experimental `$ dune init` command. This command is used to create or
  update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder)

- Experimental Coq support (fix ocaml/dune#1466, @ejgallego)

- Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix
  ocaml/dune#1973 @rgrinberg)

- Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997,
  @rgrinberg)

- Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008,
  @rgrinberg)

- Warn instead of failing if an opam file fails to parse. This opam file can
  still be used to define scope. (ocaml/dune#2023, @rgrinberg)

- Do not crash if unable to read a directory when traversing to find root
  (ocaml/dune#2024, @rgrinberg)

- Do not exit dune if some source directories are unreadable. Instead, warn the
  user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg)

- Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent
  `binaries` fields would be ignored, but instead they should be combined.
  (ocaml/dune#2029, @rgrinberg)

- Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg)

- Format rules: if a dune file uses OCaml syntax, do not format it.
  (ocaml/dune#2014, fix ocaml/dune#2012, @emillon)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.