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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(jokes): minor fixes and improvements #1418

Merged
merged 8 commits into from Feb 16, 2022

Conversation

thomasheyenbrock
Copy link
Contributor

Hello Remix-Team 馃捒 馃憢

While going through the jokes-tutorial I noticed some minor errors and changes for tiny improvements, which I bundled up into this PR. I split up the changes into multiple commits, each doing one and only one thing. I tried to be descriptive in the commit messages, and again it's all about minor details. Just leave a comment if some changes or intentions behind them are not clear. 馃槉

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jan 8, 2022

Hi @thomasheyenbrock,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jan 8, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 馃コ

@zardoy
Copy link

zardoy commented Feb 9, 2022

Hey, I think it would be also awesome if you also refactor:

  1. Use && instead of ? ... : null e.g.

    remix/docs/tutorials/jokes.md

    Lines 1218 to 1220 in 8b3fc2e

    {process.env.NODE_ENV === "development" ? (
    <LiveReload />
    ) : null}
    can be shorten by removing : null part and replacing ? with &&

I think it'd much better doing here to avoid potential conflicts as its not merged yet.

  1. const [randomJoke] = await db.joke.findMany({
    Refactor findMany to findFirst (remove brackets and take: 1). It'd would be extremely useful to know for new users.

@ryanflorence
Copy link
Member

Thanks for taking the time to do this but several things were updated in other PRs (like language block highlighting, etc.) creating conflicts.

A brief look at the updates, some look great, others I'd probably pass on, but Kent made the tutorial so I'll let him review.

@kentcdodds you want to take a look at which commits you'd like to take from here and @thomasheyenbrock can push again w/o merge conflicts and just the commits you'd like to keep?

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Stellar! Thank you for taking the time to improve things for us. I just made a few of my own tweaks and now we're set. Thank you!

@kentcdodds kentcdodds changed the title fix(jokes-tutorial): minor fixes and improvements docs(jokes): minor fixes and improvements Feb 16, 2022
@kentcdodds kentcdodds merged commit c8e219f into remix-run:main Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants