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

feat(cli): Add README to .redwood #8790

Merged
merged 12 commits into from
Jul 4, 2023
Merged

feat(cli): Add README to .redwood #8790

merged 12 commits into from
Jul 4, 2023

Conversation

Josh-Walker-GM
Copy link
Collaborator

Based on discussion with @thedavidprice about including a README to explain .redwood

Changes

  1. Adds a template for a readme file that describes what .redwood is and what it contains.
  2. Adds a check that fires on every CLI command to check that this new readme exists.

@jtoar Would you mind reviewing this. I think you tend to be the documentation guy and I might not have the same voice as is typical. @thedavidprice anything missing here to what you envisioned?

@Josh-Walker-GM Josh-Walker-GM added the release:feature This PR introduces a new feature label Jun 30, 2023
@Josh-Walker-GM Josh-Walker-GM added this to the v6.0.0 milestone Jun 30, 2023
@Josh-Walker-GM Josh-Walker-GM modified the milestones: v6.0.0, next-release Jun 30, 2023
@Josh-Walker-GM
Copy link
Collaborator Author

Josh-Walker-GM commented Jun 30, 2023

We could add this to the create redwood app templates too? If we are happy enough to introduce the overhead of keeping the template files in sync with the template that lives inside the cli package?

@thedavidprice
Copy link
Contributor

@Josh-Walker-GM It's much more thorough and better than I imagined! Thank you for taking the time to do this.

I don't want to miss the original impetus for making sure devs are provided a link to the telemetry page and can control their project's telemetry settings. Were you planning on addressing/adding this somewhere else? And/or what about adding a specific section to the README?

@jtoar any additional ideas or reactions?

@Josh-Walker-GM
Copy link
Collaborator Author

Happy to add a specific telemetry section. I'll see what @jtoar thinks about the readme and how best to add a specific telemetry section before I do.

With the changes to be "verbose" by default then the only real telemetry option is to disable via the REDWOOD_DISABLE_TELEMETRY env var or pass --no-telemetry every time.

I'm also adding a note which includes a link to the telemetry notice to the telemetry log file inside of .redwood/logs/telemetry.out.log

Did you, @thedavidprice, want an explicit output to the console that highlights that telemetry is enabled? If so then it's just a question of how often, only once ever or once per 24 hours?

Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

@Josh-Walker-GM nice, mainly just some typo fixes

packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
packages/cli/src/lib/templates/dotredwood.md.template Outdated Show resolved Hide resolved
@thedavidprice
Copy link
Contributor

@Josh-Walker-GM

Happy to add a specific telemetry section.

^^ Let's do it, how about:

Telemetry
RedwoodJS collects completely anonymous telemetry data about general usage. For transparency, that data is viewable in the respective directories and files. To learn more and manage your project's settings, visit [telemetry.redwoodjs.com](https://telemetry.redwoodjs.com)

With the changes to be "verbose" by default then the only real telemetry option is to disable via the REDWOOD_DISABLE_TELEMETRY env var or pass --no-telemetry every time.

^^ 👍

I'm also adding a note which includes a link to the telemetry notice to the telemetry log file inside of .redwood/logs/telemetry.out.log

^^ 👍

Did you, @thedavidprice, want an explicit output to the console that highlights that telemetry is enabled? If so then it's just a question of how often, only once ever or once per 24 hours?

^^ Nope! No need given this implementation and improved transparency + communication

@thedavidprice
Copy link
Contributor

@Josh-Walker-GM Rethinking the implementation of this (hat tip to Tobbe) — I know I wanted to find a way for this README to persist for the case of cloning and git clean. Instead of continually fs.exists and creating, there's a simpler way (maybe):

  1. create at time of yarn create redwood-app (as you had originally suggested)
  2. update .gitignore to watch (i.e. not ignore) .redwood/README.md 💡

That's much better and covers my requirements for the most part, does it not? (note: I'm aware this won't apply to existing projects — I'm ok with that as we'll need to call out the updated telemetry in the release notes for v6 and we can mention the web link there.)

What say you?

@Josh-Walker-GM
Copy link
Collaborator Author

@thedavidprice I'm fine with the change and will update accordingly.

@Josh-Walker-GM Josh-Walker-GM requested a review from jtoar July 1, 2023 00:20
@jtoar jtoar modified the milestones: next-release, v6.0.0 Jul 4, 2023
@jtoar jtoar merged commit 5ebb76c into main Jul 4, 2023
30 checks passed
@jtoar jtoar deleted the jgmw-cli/dot-redwood-readme branch July 4, 2023 07:38
jtoar pushed a commit that referenced this pull request Jul 4, 2023
Based on discussion with @thedavidprice about including a README to
explain .redwood

**Changes**
1. Adds a template for a readme file that describes what .redwood is and
what it contains.
2. Adds a check that fires on every CLI command to check that this new
readme exists.

@jtoar Would you mind reviewing this. I think you tend to be the
documentation guy and I might not have the same voice as is typical.
@thedavidprice anything missing here to what you envisioned?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants