unfoldingWord / findr

Open Components for Find and Replace
https://findrjs.netlify.app/
MIT License
0 stars 0 forks source link

README Improvements #4

Closed theNerd247 closed 1 year ago

theNerd247 commented 1 year ago

Below are some improvements that could be made to the README:

  1. remove the tree list of the packages (maybe instead point them to the packages/?)
  2. the links in the table point to npm packages - why not to the directories in the repo?
  3. You've mentioned this being a project for javascript but most of the code is written in TS. Is it reasonable for someone who doesn't have prior knowledge of TS to use your library?
  4. could you generate docs using tsdocs? The users of your libraries could benefit greatly from reference documentation
  5. Place the pnpm and nx commands under their own section heading titled "PNPM and NX Help". This lets those that are already familiar with those tools skip over that section (and can focus on the key parts of the README) and lets newcomers understand that you're giving them convenience commands on who to perform basic tasks within your monorepo without needing to dig through the nx and pnpm documentation). Also I would place the following in bold as the first line under the heading:

    All commands (and package.json scripts) should be executed from root and not from the packages directories.

  6. In "Getting started" I don't think "clone or fork this repository" is very helpful. I assume most developers would already know the basics of Git and Github by the time they discover your repo.
  7. I'm being nitpicky here...but could we remove the image title from the readme? It decreases readability of the README and makes it difficult use the README as a reference documentation.
theNerd247 commented 1 year ago

After opening #5 I've discovered some more improvements:

Reword Project Description

In my changes I've simplified the project description and added a "Features" section to give more room to advertise the library. Here are some things to consider for this section (if we decide to keep it)

  1. Find/replace is an UI feature that's been around a while and there are certain to be other tools that do this. What's different about this library compared to other common find/replace tools (answering this will make your library more discoverable).

  2. What do we mean by "Fast"? The term itself is ambiguous and could mislead users: does it mean "fast build times?", "fast search/replace over large documents?", etc? Also, if we say "fast" in some context is there some empirical evidence that we could use to backup our claims?

The same idea goes for "highly configurable"

Missing Tutorial links

Try the tutorial for one of the @findr packages:

However the links in the following text point to the npm package registry. Did you mean to say something like "each of the packages here published as separate npm packages. Here aret he links below"?

abelpz commented 1 year ago

@theNerd247

Answers to your first post:

  1. remove the tree list of the packages (maybe instead point them to the packages?)

Good suggestion, we can do that. The reason for that here is that this tree list only contains packages and not apps, so the user can clearly see which packages are included and where they are, if you feel it is redundant we can remove it.

  1. the links in the table point to npm packages - why not to the directories in the repo?

The original purpose of that table was to lead users to tutorials or walkthroughs inside the doc site, but I've only added one walkthrough for the @findr/text library, so we can change the description of that table for now, and then reintroduce it when the walkthroughs are ready.

  1. You've mentioned this being a project for javascript but most of the code is written in TS. Is it reasonable for someone who doesn't have prior knowledge of TS to use your library?

Having the library written in TS won't affect any non-TS user, every TS library is transpiled to vanilla JS as part of the building process, so the library can be used by anyone working on a TS or JS app or library. It could limit contribution to these packages, but I've found myself since I have just started using TS recently, that TS looks scarier than it actually is, and in most libraries I've been following, TS has not limited contribution.

  1. could you generate docs using tsdocs? The users of your libraries could benefit greatly from reference documentation

Yes, I have that noted as a next step, it is just that I was not as pleased with some of the types I added, and I need to add more jsdoc comments to some functions so the generated tsdocs are more useful. We could create a separate issue about this since I do believe that would be the next step on inproving documentation. And I would like this repo to be a reference for next monorepos in the ecosystem.

  1. Place the pnpm and nx commands under their own section heading titled "PNPM and NX Help". This lets those that are already familiar with those tools skip over that section (and can focus on the key parts of the README) and lets newcomers understand that you're giving them convenience commands on who to perform basic tasks within your monorepo without needing to dig through the nx and pnpm documentation). Also I would place the following in bold as the first line under the heading:

    All commands (and package.json scripts) should be executed from root and not from the packages directories.

Nice suggestion, I agree, with everything here. I think it is taken care of in your PR.

  1. In "Getting started" I don't think "clone or fork this repository" is very helpful. I assume most developers would already know the basics of Git and Github by the time they discover your repo.

I see your point, I just thought it doesn't take up much space, and I have been in that position where I didn't know any of that and everyone assumes you already know, so I did want to give that someone the chance to learn. If someone already knows the will just skip that small bit.

  1. I'm being nitpicky here...but could we remove the image title from the readme? It decreases readability of the README and makes it difficult use the README as a reference documentation.

I agree, I was trying something there, but it definitely takes up too much space, with no gain, at least not worthy gain. Let's remove it.

abelpz commented 1 year ago

@theNerd247 answer to your second post:

Yes, that is all right! Thank you for helping me see that. Instead of fast, I would say something that communicates that these libraries were thought to be used by developers who need to create find and replace UIs quickly, so it already contains every basic configuration that most find-and-replace UI components have out there (Ignore Casing, Match whole words, Use regular expressions, Preserve Case), and every library in here is a piece that can be used either separately or all together, so the developer only needs the GUI they don't need to bring in the logic, or maybe they only need the state management to build their own UI they have that in a separate library.

theNerd247 commented 1 year ago

Sounds great! Thanks! I'll make the appropriate updates in my PR

theNerd247 commented 1 year ago

I've found another issue:

$ npx nx connect-to-nx-cloud

 >  NX   ✅ This workspace is already connected to Nx Cloud.

   This means your workspace can use computation caching, distributed task execution, and show you run analytics.
   Go to https://nx.app to learn more.

   If you have not done so already, please claim this workspace:
   https://nx.app/orgs/workspace-setup?accessToken=...

This is what I get upon the first time running this command. Was this something that is only supposed to be run once universally? That is did CI or yourself already setup a workspace for this repo? If so, it might be worth removing this command from the README.

abelpz commented 1 year ago

@theNerd247 good catch, yes it is only run once, and it is already done, this should be removed from the README.

theNerd247 commented 1 year ago

Here's another thought:

I would really advertise how findr is different from regex as most developers will want to know why you're addressing a problem that seems to have already been solved (regex solves the problem of find and replace).

Maybe an idea would be to show a sample use case in trying to do "find and replace" without your library and what it looks like with your library.

Something else to consider is when your library would not be appropriate for your users.

abelpz commented 1 year ago

@theNerd247 yes that's true, this is not a substitute for regex this is actually built on top of regex, and meant to be the logic for a GUI for find and replace.

The idea for this is to make it easier to build these Graphic User Interfaces. The results that you get from it are formatted in a way that is useful for developers to display a list of results to a final user, and contains useful pointers to those results so that the user can just select the results they want to replace or navigate to them. Also allows the user to keep an updated list of results every time they do a replacement, etc.

I like the idea of creating the example cases, however, I wouldn't try to make a findr/regex comparison since it is not supposed to replace regex.

theNerd247 commented 1 year ago

Thanks! I'll update the readme to reflect this.

theNerd247 commented 1 year ago

Another note: the readme does not make any mention of PERF. We should add it to the features available

theNerd247 commented 1 year ago

Another note: add a "copy/paste" example in the readme for new users to get setup quickly