Closed markuswntr closed 3 years ago
Hey @markuswntr Thank for the PR. Big fan of Swift Numerics.
Take a look at https://github.com/markuswntr/swift-numerics/pull/1 - I've made a few improvement suggestions to the entire README based on tech writing experience - hope you like them and we can bundle the changes in this PR: https://github.com/apple/swift-numerics/pull/150:
Summary of suggestions:
- Swift Numerics modules are fine-grained; if you need support for Complex
+ Swift Numerics modules are fine-grained. For example, if you need support for
+ Complex numbers ...
- ... import ComplexModule¹ without pulling in everything else in the library:
+ ... import ComplexModule¹ as a standalone module:
...
- as well.
...
[Example in code blocks]
- // All swift-numerics API is now available
+ // The entire `swift-numerics` API is now available
- Because we intend to make it possible to adopt Swift Numerics modules in the
- standard library at some future point...
+ The Swift Numerics project is intended to be adopted in the standard library
+ in future. Therefore, it...
1... 2... 3...
) for the Using Swift Numerics in your project section, since it's a sequential process - this helps with UX- Swift Numerics is a standalone library separate from the core Swift project. In
+ Swift Numerics is a standalone library that is separate from the core Swift project. In
1. About [`RealModule`](Sources/RealModule/README.md)
- [`ApproximateEquality`](Sources/RealModule/ApproximateEquality.swift)
2. About [`ComplexModule`](Sources/ComplexModule/README.md)
Let me know what you think.
Cheers!
Thanks to @8bitmp3, this PR now address not only the approximate equality module but also spelling/grammar - so I have changed the title to reflect the changes.
I understand that line wraps aren't too user-friendly @xwu They are part of the docs-as-code approach and help users see "the end of the line", since Markdown rendering can't always auto-wrap depending on where you view the files. This linting style has been adopted by certain well-known open source Google projects (they even include it in doc tests).
They are part of the docs-as-code approach
I'm confused by this claim; as @xwu called out, wrapping at 80-cols makes diffs much larger than they should be for minor changes, which makes code review harder and more error-prone. This seems to be contrary to "docs-as-code" (one of the things code should permit is easily seeing how it has changed over time, as well as reviewing changes).
Markdown rendering can't always auto-wrap
Which renderers can't? Certainly whatever Github uses to render for the web does, and so does Xcode, which are by far the two renderers that will be most commonly used. Are there specific renderers that don't, which we expect to be frequently used with the Swift Numerics documentation (and are there reasons why viewing the docs on the web is not a viable option in those cases?)
@stephentyrone I understand where you and @xwu come from and the GitHub web UI helps to easily the entire text, while applying soft-wrapping and other user-friendly features.
I can revert the wrapping changes back so the diffs are more visible - whatever makes it easier for you to review here 👍
Which renderers can't?
I've reviewed code and docs in VSCode and used various IDEs - from IDEA to Xcode - and I'm aware some folks don't always have the default settings set to "auto-wrap": "on". However, many things in modern IDEs are automated by default these days, including the awesome Xcode.
So, for instance, if a contributor forks a repo, opens a Markdown file, and decides to make certain changes, they may see some long lines that make up long paragraphs, such as:
For some background, there's a GitLab blog post about the pros and cons of wrapping text, that summarizes this quite well:
Hi @markuswntr, can you update this to point against main instead of master? Thanks.
Hi @stephentyrone, updated base branch to main. Thanks!
Approximate Equality has recently been merged into master. This PR updates the README.md to reflect the changes. While it is part of the
RealModule
it really is an extension onNumeric
, so I decided to list it below theRealModule
but separately.