Skip to content

Export SUPPORTS_SHARED_LIBRARIES in ocamlc -config#1691

Merged
dra27 merged 1 commit intoocaml:trunkfrom
dra27:support-dynamic-linking-config
Sep 13, 2018
Merged

Export SUPPORTS_SHARED_LIBRARIES in ocamlc -config#1691
dra27 merged 1 commit intoocaml:trunkfrom
dra27:support-dynamic-linking-config

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Apr 2, 2018

Exposes SUPPORTS_SHARED_LIBRARIES from Makefile.config as shared_libraries in ocamlc -config

@dra27 dra27 mentioned this pull request Apr 2, 2018
Merged
@gasche
Copy link
Copy Markdown
Member

gasche commented Apr 2, 2018

I support the feature and believe the implementation is correct. Do we get five minutes to discuss whether supports_shared_libraries isn't a more descriptive name (in term of suggesting booleanness) than shared_libraries?

(supports_shared_libraries is what ocamlbuild's ocamlbuild_config.ml uses, but I think there the idea was just to directly reuse the upstream name rather than any conscious naming decision.)

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Apr 2, 2018 •
edited
Loading

I um'd and ah'd over the name (it has two names in our code base too - it's different in s.h!). I'm not attached to the name at all, although other booleans (e.g. windows_unicode, flamda) have similarly non-Boolean names.

I'm in no particular rush for this to be merged, so happy to wait for other additional (!) opinions on suitable naming.

@mshinwell
Copy link
Copy Markdown
Contributor

mshinwell commented Apr 10, 2018

I tend to think it would be better to keep this patch consistent as regards naming, so the config dump should be "supports_shared_libraries". @dra27 Can you make this change and then merge?

gasche
gasche approved these changes Apr 10, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@gasche gasche left a comment

Choose a reason for hiding this comment

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

(independently of the naming discussion, here is a magical seal of approval.)

@gasche gasche mentioned this pull request Apr 10, 2018
Merged
@dra27 dra27 force-pushed the support-dynamic-linking-config branch from ff9f818 to 4aabef3 Compare September 11, 2018 13:17
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Sep 11, 2018

@mshinwell - (finally) done. I'll merge when CI passes.

@dra27 dra27 merged commit 6fab940 into ocaml:trunk Sep 13, 2018
@dra27 dra27 deleted the support-dynamic-linking-config branch September 13, 2018 12:40
@dra27 dra27 mentioned this pull request Nov 23, 2018
Merged
@dbuenzli dbuenzli mentioned this pull request Jul 30, 2020
Closed
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
@leostera
Package Search UX Improvements (ocaml#1691)
d28e89c 
* autofocus search input on package search results page
* on package overview page: add tabindex="1" to in-package search input
* move package search input in top navbar earlier in the DOM so it gets tab focus earlier
* add OpenSearch manifest for package search
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gasche gasche gasche approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dra27 @gasche @mshinwell