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

Fix tailwindcss setup #1407

Merged
merged 11 commits into from Oct 29, 2020
Merged

Conversation

mohinderps
Copy link
Contributor

Closes #1301

This is draft PR, which fixed tailwindcss setup command. Feel free to review this and suggest the changes that need to done.

@thedavidprice thedavidprice self-requested a review October 28, 2020 03:26
Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

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

🚀

@thedavidprice
Copy link
Contributor

@mohinderps Woah, this is a ton of work and the functionality is 💯 Thank you for taking the time to get this one right. It's so helpful.

There's an incoming PR #1408 that will add an option to this command. The plan is to merge this first. However, I do want to loop in the author of #1408 to see what you've done and give any feedback before I merge (he's a great guy and has been helpful with Redwood since March). Once we have a coordinated plan, I'll merge. I expect this should be a quick turnaround.

Just let me know if you have any questions.

@thedavidprice
Copy link
Contributor

Looping in @olance who's working on extending the command with an option via #1408

I suggested a plan for us to discuss:

Open to more discussion as needed. And, most of all, thank you both!

@mohinderps
Copy link
Contributor Author

@mohinderps Woah, this is a ton of work and the functionality is 💯 Thank you for taking the time to get this one right. It's so helpful.

Thank you @thedavidprice for such kind words. You have been of great help to me. I just followed your directions.
Looking forward to contribute more in this project.

And also if everything looks fine to you, can you also add hacktoberfest-accepted label ? So that this is counted as my hacktoberfest contribution.

@thedavidprice
Copy link
Contributor

re: hacktoberfest-accepted
If we merge the PR this week, it'll count without the label! And I’ll make sure we add the label otherwise. Thanks for asking.

@mohinderps
Copy link
Contributor Author

Cool @thedavidprice !

@olance
Copy link
Contributor

olance commented Oct 28, 2020

👋

Thanks for the ping @thedavidprice :)
I'll try to review this today!

Copy link
Contributor

@olance olance left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍
Just added a suggestion on renaming tailwindImportsDoesExist to tailwindImportsExist, which does simplify the code a little bit as well as fixing a grammatical error ^^

packages/cli/src/commands/setup/tailwind/tailwind.js Outdated Show resolved Hide resolved
packages/cli/src/commands/setup/tailwind/tailwind.js Outdated Show resolved Hide resolved
thedavidprice and others added 2 commits October 28, 2020 17:26
Co-authored-by: Olivier Lance <olance@users.noreply.github.com>
Co-authored-by: Olivier Lance <olance@users.noreply.github.com>
@thedavidprice
Copy link
Contributor

Merging this now. Thanks again @mohinderps 🚀

@olance ready for you to take over in #1408 🔥

@thedavidprice thedavidprice merged commit c2d7860 into redwoodjs:main Oct 29, 2020
@thedavidprice thedavidprice added this to the Next release milestone Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yarn rw setup tailwind fails if packages already installed or if imports exist in index.css
3 participants