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: generate index page dynamically based on selected packages #208

Closed
wants to merge 4 commits into from
Closed

feat: generate index page dynamically based on selected packages #208

wants to merge 4 commits into from

Conversation

CarlosGomez-dev
Copy link
Contributor

Generate index page dynamically based on selected packages

  • I reviewed linter warnings + errors, resolved formatting, types and other issues related to my work
  • The PR title follows the convention we established conventional-commit
  • I performed a functional test on my final commit

This resolves #190, home component is now created dynamically based on the selected packages, this allows us to have a base index and a tailwind index, both are then modified according to the selected packages.

Looked into codemods and jscodeshift, but they seem a bit too complex for this problem, so I resorted to simply replacing strings. I'm open to suggestions for a simpler way to do it.


@juliusmarminge
Copy link
Member

@ochicf How far off are you into getting codemods working? Seems redundant to do this now if we soon are moving to codemods anyways.

@ochicf
Copy link
Contributor

ochicf commented Jul 19, 2022

@ochicf How far off are you into getting codemods working? Seems redundant to do this now if we soon are moving to codemods anyways.

@juliusmarminge I was focusing on attempting the reinstalls, so basically going deep instead of going wide. If codemods is chosen as the tool we want to use event for regulars installs (which sounds it does) then I could focus on writing them and refactor the code. Not sure on ETA as I'm pretty new with it and I expect encountering walls, so it won't be in a matter of days.

IMHO, the modifications done in this MR would be better done with a template engine (if not considering codemods) as parsing code with replace it's not the safest nor the most maintainable. With that said: if this fixes an issue that is causing problems, then go for it and we will refactor it with codemods anyway.

@juliusmarminge
Copy link
Member

juliusmarminge commented Jul 19, 2022

If codemods is chosen as the tool we want to use event for regulars installs (which sounds it does) then I could focus on writing them and refactor the code

I am more than happy to jump in and work on some parts of this. Is the basic transforms done? In that case we can merge them in here somewhere and I (and perhaps others) can start working on some scaffolding stuff maybe?

IMHO, the modifications done in this MR would be better done with a template engine (if not considering codemods) as parsing code with replace it's not the safest nor the most maintainable.

Yea this was my concern. Feels quite fragile and hard to expand upon.

With that said: if this fixes an issue that is causing problems, then go for it and we will refactor it with codemods anyway.

I don't think there being a few missing cards on some of the templating options is that big of a deal for us to go in this direction just for a short time before migrating to codemods.

Thank you for the contribution, but I'll close this for now and await codemods. Feel free to help out when we get to that point :)

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.

enhancement: index.tsx doesn't include the correct cards
3 participants