Chore/nx migration #3715
Chore/nx migration #3715
Conversation
There was a problem hiding this comment.
The routes unit tests are too complex and go beyond the responsibility of routes. Can't just comment them out of course. If user engagement (e.g. clicking something) is needed to get to a routes expectation, that's a smell.
Need to sure what the routes tests are covering is tested somewhere.
| "import/newline-after-import": "error", | ||
| "import/no-named-as-default": "off", |
| @@ -40,15 +38,6 @@ | |||
| "accessorPairPositioning": "getThenSet" | |||
| } | |||
| ], | |||
| "import/no-duplicates": "error", | |||
There was a problem hiding this comment.
the import plugin was preventing nx from running. (maybe due to commonjs?)
| "plugin:import/recommended", | ||
| "plugin:import/typescript", |
There was a problem hiding this comment.
the import plugin was preventing nx from running. (maybe due to commonjs?)
.eslintrc.base.json
Outdated
There was a problem hiding this comment.
Reasoning for this being a seperate file?
| work correctly both with client-side routing and a non-root public URL. | ||
| Learn how to configure a non-root public URL by running `npm run build`. | ||
| --> | ||
| <title>Precious Plastic Community</title> |
There was a problem hiding this comment.
I think we have a script that replaces this before serving, but it's something to confirm.
| href="https://firebasestorage.googleapis.com" | ||
| crossorigin | ||
| /> | ||
| <noscript><link rel="stylesheet" href="path/to/stylesheet.css" /></noscript> |
| <meta property="og:title" content="Community Platform" /> | ||
| <meta | ||
| property="og:description" | ||
| content="A series of tools for the Precious Plastic community to collaborate around the world. Connect, share and meet each other to tackle plastic waste." |
| content="A series of tools for the Precious Plastic community to collaborate around the world. Connect, share and meet each other to tackle plastic waste." | ||
| /> | ||
| <meta property="og:image" content="./social-image.jpg" /> | ||
| <meta property="og:url" content="https://community.preciousplastic.com" /> |
| <meta name="twitter:title" content="Community Platform" /> | ||
| <meta | ||
| name="twitter:description" | ||
| content="A series of tools for the Precious Plastic community to collaborate around the world. Connect, share and meet each other to tackle plastic waste." |
| import { trackEvent } from '../../../../common/Analytics' | ||
| import { useCommonStores } from '../../../../common/hooks/useCommonStores' | ||
| import { Breadcrumbs } from '../../../../pages/common/Breadcrumbs/Breadcrumbs' | ||
| import { seoTagsUpdate } from '../../../../utils/seo' |
There was a problem hiding this comment.
General point, just picking this as an example. The depth of these relative paths is annoying. No other option?
There was a problem hiding this comment.
Should be fixed by splitting up the code in libraries. ex:
import { Breadcrumbs } from '@onearmy.apps/shared'
mariojsnunes
force-pushed
the
chore/nx-migration
branch
from
2e6bf53
to
4f028ca
Compare
|
thanks @benfurber will consider all your comments on the next try. Important info from discord:
Abandoning this PR as I found a few components that are coupling things up too much:
fixing them first before proceeding with the migration (as fixing them means changing code, not just moving things around) |
PR Checklist
PR Type
Description
What this PR does
Git Issues
Closes #
Screenshots/Videos
If useful, provide screenshot or capture to highlight main changes
What happens next?
Thanks for the contribution! We try to make sure all PRs are reviewed ahead of our monthly maintainers call (first Monday of the month)
If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.
If you need more immediate feedback you can try reaching out on Discord in the Community Platform
developmentchannel.