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

Update README.md #452

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions README.md
Expand Up @@ -25,7 +25,7 @@ It aims to conform to [RFC 7159](https://tools.ietf.org/html/rfc7159).
Building on Unix with `git`, `gcc` and `autotools` <a name="buildunix"></a>
--------------------------------------------------

Home page for json-c: https://github.com/json-c/json-c/wiki
Homepage for json-c: https://github.com/json-c/json-c/wiki

### Prerequisites:

Expand Down Expand Up @@ -72,7 +72,7 @@ $ make USE_VALGRIND=0 check # optionally skip using valgrind
Install prerequisites <a name="installprereq"></a>
-----------------------

If you are on a relatively modern system, you'll likely be able to install
If you are in a relatively modern system, you'll likely be able to install
Copy link
Member

Choose a reason for hiding this comment

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

"in" is a rather odd way to word this

the prerequisites using your OS's packaging system.

### Install using apt (e.g. Ubuntu 16.04.2 LTS)
Expand All @@ -84,9 +84,9 @@ sudo apt install valgrind # optional

Then start from the "git clone" command, above.

### Manually install and build autoconf, automake and libtool
### Manually install and build autoconf, automake, and libtool

For older OS's that don't have up-to-date version of the packages will
For older OS's that don't have an up-to-date version of the packages will
Copy link
Member

Choose a reason for hiding this comment

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

"don't have up-to-date versions" would be better

require a bit more work. For example, CentOS release 5.11, etc...

```sh
Expand Down Expand Up @@ -131,7 +131,7 @@ default. You may turn it on by adjusting your configure command with:
--enable-threading

Separately, the default hash function used for object field keys,
lh_char_hash, uses a compare-and-swap operation to ensure the randomly
lh_char_hash, uses a compare-and-swap operation to ensure the random
seed is only generated once. Because this is a one-time operation, it
is always compiled in when the compare-and-swap operation is available.

Expand All @@ -151,7 +151,7 @@ CMake can take a few options.

Variable | Type | Description
------------------|------|--------------
BUILD_SHARED_LIBS | Bool | The default build generates static library. Enable this to generate shared (dll/so) library.
BUILD_SHARED_LIBS | Bool | The default build generates a static library. Enable this to generate shared (dll/so) library.
Copy link
Member

Choose a reason for hiding this comment

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

er.. I'm not even sure this description is accurate, and if it is, then it shouldn't be. The default definitely should not be to just build a static library.

Copy link
Contributor

Choose a reason for hiding this comment

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

By default, cmake will build one or the other. If BUILD_SHARED_LIBS is ON, then cmake will create a shared library, but not a static library. There are ways to change that behavior though, with only minor editing of CMakeLists.txt apparently. Some info about it here: https://stackoverflow.com/questions/2152077/is-it-possible-to-get-cmake-to-build-both-a-static-and-shared-version-of-the-sam

ENABLE_RDRAND | Bool | Enable RDRAND Hardware RNG Hash Seed
ENABLE_THREADING | Bool | Enable partial threading support

Expand Down