Rewrite README file #111
Rewrite README file #111
Conversation
There was a problem hiding this comment.
Made a few suggestions. I like the notes you left to yourself.
I am definitely looking forward to the docs on pre-commit and how to run GitHub Actions locally. If our Spaces environment comes already prepared for those workflows, even better.
One suggestion: it's probably true that more users will just want to consume and build Beman libraries than make changes to them. Let's consider creating a CONTRIBUTING.md file and putting instructions in there that you wouldn't need if you were happy just building a release of this repo.
README.md
Outdated
| This project pulls [Google Test](https://github.com/google/googletest) | ||
| from GitHub as a development dependency, | ||
| you can disable this behavior by setting `BEMAN_EXEMPLAR_BUILD_TESTS` to `OFF`. |
There was a problem hiding this comment.
Please remind me to update this section of the README whenever we decide on an approach to make the http-based dependency resolution optional.
Yeah one of the main goal is to minimize the amount of replacement we need for the initial setup. We don't have to be exhaustive with those. |
@bretbrownjr Unfortunately it's really tricky to run GitHub Actions locally, you have to setup docker and cross-platform emulation if you want to test windows/ macos based workflows, there's only community maintained solutions. See: https://github.com/nektos/act I do get this question a lot, maybe I should document this. |
wusatosi
requested review from
camio,
dietmarkuehl,
neatudarius and
steve-downey
as code owners
README.md
Outdated
| ```shell | ||
| # Assume /opt/beman.exemplar staging directory. | ||
| $ c++ -o identity_usage examples/identity_usage.cpp \ | ||
| -I /opt/beman.exemplar/include/ \ | ||
| -L/opt/beman.exemplar/lib/ -lbeman.exemplar | ||
| ### Produce beman.exemplar static library locally | ||
|
|
||
| You can include exemplar's headers locally | ||
| by producing a static `libbeman.exemplar.a` library. | ||
|
|
||
| ```bash | ||
| cmake --workflow --preset gcc-release | ||
| cmake --install build/gcc-release --prefix /opt/beman.exemplar | ||
| ``` | ||
|
|
||
| </details> | ||
| This will generate such project structure at `/opt/beman.exemplar`. | ||
|
|
||
| <details> | ||
| <summary> Use beman.exemplar directly from CMake </summary> | ||
| ```txt | ||
| /opt/beman.exemplar | ||
| ├── include | ||
| │ └── beman | ||
| │ └── exemplar | ||
| │ └── identity.hpp | ||
| └── lib | ||
| └── libbeman.exemplar.a | ||
| ``` |
There was a problem hiding this comment.
We should just remove this section. We don't want to support arbitrary -lbeman.exemplar and such. This is especially true for libraries that will actually have link time dependencies for release builds.
Install instructions are good to document, but that would be for people maintaining these packages in Conan, vcpkg, Debian, or a similar project. They would go in a packaging guide, if we wanted to support one of those.
There was a problem hiding this comment.
I agree with this, I will go ahead and remove this section.
There was a problem hiding this comment.
Actually, I am not sure, cmake --install is a command we maintain, should I document this differently?
| You will also need to add `beman::exemplar` | ||
| to the link libraries of any libraries or executables that include `beman/exemplar/*.hpp` in their source or header file. | ||
| You will also need to add `beman::exemplar` to the link libraries of | ||
| any libraries or executables that include beman.exemplar's header file. |
There was a problem hiding this comment.
Looks good.
Note that it would be a little tricky to get the pluralization of "header file" versus "header files" correct if a project template contained this text. Some projects would be one header like beman.exemplar, but many would not.
README.md
Outdated
| ### Linking your project to beman.exemplar with compiler options | ||
|
|
||
| Build systems that support `pkg-config` by providing a `beman.exemplar.pc` file. | ||
| Build systems that support interoperation via `pkg-config` should be able to detect `beman.exemplar` for you automatically. | ||
| Compile your project so that it links to the header and library correctly. | ||
|
|
||
| </details> | ||
| ```bash | ||
| c++ -I/opt/beman.exemplar/include \ | ||
| -L/opt/beman.exemplar/lib -lbeman.exemplar \ | ||
| your_project.cpp -o your_project | ||
| ``` |
There was a problem hiding this comment.
Again, I don't think we should bother documenting how to support direct compiler invocations. I suggest we remove this section.
|
To the concerns of @wusatosi about how much work is required to template this repo, the bulk of the README is not particularly unique to I'm happy to see this PR land as-is, but a future PR to slim this README down a lot and point users at a common guide should be what we're looking to do. Though we could consider that for the future. It's possible we need a robust implementation of some Beman tooling infrastructure of some sort first. |
Co-authored-by: Bret Brown <mail@bretbrownjr.com>
There was a problem hiding this comment.
This looks like a good improvement overall so I'm approving this. @wusatosi lets get some of these PRs closed out -- you've been doing some great work here and I'd like to see it in the baseline :)
Fix #97 .
Please squash merge.
Major painpoints in current README:
pre-commit, CMake preset,codespace)This PR aim to fix these issues.
Major TODOs
Document developer facing tool:pre-commitDocument developer facing tool: Current GitHub Action workflow and how to update them.Developer facing tool would be moved to
docs/, see #120 .This PR also adds spell checks.