-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
All in with monorepo #5409
All in with monorepo #5409
Conversation
✅ Deploy Preview for volto canceled.
|
* main: Updated Readme.md (#5401) Transfer of main Plone types definitions to its own package (#5397) Fix reference link to configuration/settings (#5410) Docs: remove orphans and move branch, version, and support policies i… (#5385) Add documentation chapter to Volto contributing (#5377) Release generate-volto 8.0.2 Add postcss-less to the default addon (#5408)
.vscode/settings.json
Outdated
@@ -0,0 +1,23 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file intended to be here? I see some customizations, which may not be to everyone's liking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The colors? :) It is required for the eslint config to get it right. Don't know if that config can be "global". I will remove the colors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, The colors are only "the shell" of VScode customizations though, using the Peacock official extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed! no longer needed.
@plone/volto-team I think this is ready. All is green, except the Towncrier checks will fail because it does not have "reference" with main. Once it's merged, it will be fine. I'd like to merge as soon as possible because it's already a mess to look at the changes, since it detects more than 3000 and Github file diff view just dies. We can refine in subsequents PRs. I've tested to merge PRs and all file movements are detected well. So existing PRs will behave correctly (the merge has to be done via command line, locally). We'll have to see how difficult the backports to 16 are. We could also move 16 build to a monorepo approach, shouldn't be that hard, and pretty straightforward following the changes in main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sneridagh When I switched to this branch to take a look, I got an error from pnpm:
% git checkout gotomonorepo
branch 'gotomonorepo' set up to track 'origin/gotomonorepo' by rebasing.
Switched to a new branch 'gotomonorepo'
Changes to lockfile found, running `pnpm install`
ERROR Unknown option: 'prefer-frozen-shrinkwrap'
Did you mean 'prefer-frozen-lockfile'? Use "--config.unknown=value" to force an unknown option.
For help, run: pnpm help install
Running pnpm pnpm install,--prefer-offline,--prefer-frozen-shrinkwrap,--no-optional failed
apps/plone/package.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of adding apps/plone to the repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's part of the reorganization, as you already read in the docs, the idea of the apps
folder is having all the support apps in there as in docs
, api
.
The other idea is a playground with demostration of frameworks using the libraries, ideally having them tested too in CI.
If so, having a Volto project in there, and being able to test it in full seems "natural" to have it too. Also to test new things in there, like in the near future pnpm
for projects too, or play with the idea of having a more "light" project setup for Volto as a library, without relying on Razzle, something I discussed so many times with @tiberiuichim
This will allow also a better CI story on Volto projects too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mentioned this in a private conversation -- I think we'll need to follow up soon with some checks to make sure apps/plone
doesn't fall out of sync with the project generator.
@@ -123,7 +121,7 @@ Run "npm install -g @plone/generator-volto" to update.`, | |||
|
|||
if (!this.voltoYarnLock) { | |||
this.log(chalk.red("Retrieving Volto's yarn.lock")); | |||
this.voltoYarnLock = await utils.getVoltoYarnLock(voltoVersion); | |||
// this.voltoYarnLock = await utils.getVoltoYarnLock(voltoVersion); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the generator still need some work? We shouldn't be logging that it is retrieving yarn.lock if that's not what it is doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I forgot about this one. My gut feeling is it should be removed, we force every time to create a new lock file on the project. It's not necessarily wrong, as long as in CI we test it. Although we could think in adding more tests to ensure that all is in place (eg. linting and prettifying) and we don't have hoisting problems (the fact that it does start and tests pass it's more than enough, but not complete from the DX point of view).
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, the important thing is that it works and that we have tests to make sure it keeps working.
Updated |
* main: Update yarnhook to 0.6.1 in order to support pnpm 8 (#5444) Special breadcrumb in controlpanel (#5292) Adding extraction of videoID from youtube videos url which contain '/live/' (#5426) Release 18.0.0-alpha.2 Release generate-volto 8.0.3 Changelog Bring back yeoman install which in v5 is deprecated (#5438) Improvements and completeness of the ContentMetadataTags component (#5433)
* main: Move dangling news item to the right place Better start script in root package.json, enable and better config for husky/lint-staged All in with monorepo (#5409) changed toNumber to parseFloat #5447 (#5448) Release 18.0.0-alpha.3 Changelog Revert "Improvements and completeness of the ContentMetadataTags component (#5433)" (#5449) Update yarnhook to 0.6.1 in order to support pnpm 8 (#5444) Special breadcrumb in controlpanel (#5292) Adding extraction of videoID from youtube videos url which contain '/live/' (#5426) Release 18.0.0-alpha.2 Release generate-volto 8.0.3 Changelog Bring back yeoman install which in v5 is deprecated (#5438) Improvements and completeness of the ContentMetadataTags component (#5433) Use container from component registry in sitemap component and also refactor the class to functional component(#5418) (#5420) Fix image paths in development mode (#5429) Visible focus semantic UI - reset (#5335) Remove mention of LTS in Volto (#4905) JSX is now in Pygments (#5412)
* main: Move dangling news item to the right place Better start script in root package.json, enable and better config for husky/lint-staged All in with monorepo (#5409) changed toNumber to parseFloat #5447 (#5448) Release 18.0.0-alpha.3 Changelog Revert "Improvements and completeness of the ContentMetadataTags component (#5433)" (#5449)
* main: (21 commits) Cleanup and renaming (#5456) Move dangling news item to the right place Better start script in root package.json, enable and better config for husky/lint-staged All in with monorepo (#5409) changed toNumber to parseFloat #5447 (#5448) Release 18.0.0-alpha.3 Changelog Revert "Improvements and completeness of the ContentMetadataTags component (#5433)" (#5449) Update yarnhook to 0.6.1 in order to support pnpm 8 (#5444) Special breadcrumb in controlpanel (#5292) Adding extraction of videoID from youtube videos url which contain '/live/' (#5426) Release 18.0.0-alpha.2 Release generate-volto 8.0.3 Changelog Bring back yeoman install which in v5 is deprecated (#5438) Improvements and completeness of the ContentMetadataTags component (#5433) Use container from component registry in sitemap component and also refactor the class to functional component(#5418) (#5420) Fix image paths in development mode (#5429) Visible focus semantic UI - reset (#5335) Remove mention of LTS in Volto (#4905) ...
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Note for Volto core developers (/cc @plone/volto-team)
Once this is merged, please take a look at: https://github.com/plone/volto/blob/a5401295af0a4fb799401102788ab446f4ad6276/docs/source/contributing/developing-core.md
It is recommended that you remove your
node_modules
from your local Volto cloned repository, then runpnpm i
. You cannot run the repo with other thing thanpnpm
anymore.If you have any PR opened these days, you'll have to update it using the command line.
git
is able to detect the moves, but you probably will have to confirm them. Also, the Voltonews
items now live inpackages/volto/news
butgit
is also detecting them.We will be improving and polishing in the next days in case that there's something still not adjusted.