farmOS / farmOS-map

farmOS-map is an OpenLayers wrapper library designed for agricultural mapping needs. It can be used in any project that has similar requirements.
https://farmOS.github.io/farmOS-map
MIT License
35 stars 21 forks source link

Link to NPM section under Usage section in README.md. #188

Closed mstenta closed 1 year ago

mstenta commented 1 year ago

For someone reading the README.md and coming with an expectation that they can use farmOS-map in an NPM/Webpack project, it isn't immediately clear in the "Usage" section that they can do that. I had to Ctrl+F "npm" to find the section. I propose we link to it from "Usage".

jgaehring commented 1 year ago

Oh yea, this is a really good call. I think most JS devs (and perhaps non-JS devs?) come to expect full installation options and instructions to come at the top of a README, if nothing else just to know the proper namespace of the npm package.

I'd maybe go one step further and propose we pull those details up to the top, and give them their own h2 heading ## Installation, subdivided with ### As a <script> tag and ### As an npm package headings. The Webpack details can still remain further down in the README, with a link such as you've proposed, @mstenta. That would be everything starting from:

Finally, it is recommended to externalize farmOS-map such that it is not actually bundled by Webpack.

But the npm details, though related, should be treated separately, b/c they're not strictly relevant to whether someone is using Webpack or not.

Happy to tack on another commit to this PR if others agree.

mstenta commented 1 year ago

I like that idea @jgaehring - the only minor hesitation I have is a desire to keep the "Usage" section as brief as possible. Linking to the more detailed webpack/npm considerations felt like a simple way to maintain that. But open to other opinions on this! @symbioquine / @paul121 ?

jgaehring commented 1 year ago

Oh, I was actually suggesting the "Installation" be place outside of and before the "Usage" section. That is actually the convention with most JS projects. Your point still stands, though, if it's more about keeping critical "Usage" instructions closer to the top.

jgaehring commented 1 year ago

And it would be just the stuff relevant to the npm package, specifically these 4 lines, which hopefully won't change much over time with respect to maintenance:

farmOS-map can be added via NPM or similar package managers. e.g.

npm install @farmos.org/farmos-map@2

Then farmOS-map can be accessed using an import statement.

import { MapInstanceManager, projection } from '@farmos.org/farmos-map';

Then maybe a link. Or really even just the first two lines. B/c that is such a critical preliminary to any usage that might follow, imo. I myself get frustrated when I just can't even find the npm package name in docs, where it's buried deep in the usage notes. The main imports are nice, too, but should more or less follow once you know the npm namespace, so less critical.

mstenta commented 1 year ago

That sounds good to me! Feel free to add a commit! Thanks @jgaehring !!

jgaehring commented 1 year ago

That sounds good to me! Feel free to add a commit!

Added via 9eba4ce. Kept it as minimal as possible. Let me know what you think, otherwise, all looks good to me!

mstenta commented 1 year ago

LGTM!