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

Fixed typo and version mismatch in upgrade guide Volto 17 #5703

Closed
wants to merge 1 commit into from

Conversation

ksuess
Copy link
Member

@ksuess ksuess commented Jan 26, 2024

Fix #5702

Copy link

netlify bot commented Jan 26, 2024

Deploy Preview for volto ready!

Name Link
🔨 Latest commit 883a853
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/65b3878a84e24c0008286bd1
😎 Deploy Preview https://deploy-preview-5703--volto.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 26, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 883a853
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/65b3878a8402870008f0bf03

@ichim-david ichim-david self-requested a review January 27, 2024 08:51
@stevepiercy
Copy link
Collaborator

First @ksuess is correct to update this discrepancy. But I have to ask, is there a better way that reduces maintenance of the docs and gives developers an easier way to update their code? Why, yes, there is.

I chatted with @ichim-david in Discord. I think it's too much work to maintain dependencies as diffs in documentation every time we update the deps in Volto. Additionally copy-pasting a diff into one's package.json is not efficient. And the diffs in the docs conflict with one another, as @ksuess shows in this PR.

It is easier to go to the source file in the repo for that branch, copy-paste that into your package.json, then run a diff to see what has changed. When I want to update my dev deps for a 17.x.x site, I go here:

https://github.com/plone/volto/blob/17.x.x/packages/generator-volto/generators/app/templates/package.json.tpl#L139-L161

Select "Copy lines" from the stoplight menu, and paste into my package.json. Diff, tweak, and done.

I'd suggest we remove the diffs, and instead point to the source code in the repo and add text, such as the following.


Dependencies in Volto 17.x.x will continue to be updated after this change note is published.
To update your project against the current release of Volto 17, you should compare the development dependencies in your package.json against those in Volto's 17.x.x branch's package.json.

@sneridagh
Copy link
Member

@stevepiercy the upgrade guide ones are correct, and since the docs are "frozen" in time, they are correct. They are not going to change overtime without another breaking.

The others (the ones in the typescript.md) it's arguable they should point to the current latest stable generator, which is tested per version right now, so I guess it's ok.

@stevepiercy
Copy link
Collaborator

@stevepiercy the upgrade guide ones are correct, and since the docs are "frozen" in time, they are correct. They are not going to change overtime without another breaking.

@sneridagh do you mean that @ksuess should revert the changes she made to docs/source/upgrade-guide/index.md? A code review would make it clear.

The others (the ones in the typescript.md) it's arguable they should point to the current latest stable generator, which is tested per version right now, so I guess it's ok.

OK, so @ksuess should replace the diff in docs/source/contributing/typescript.md with the text in my comment below the horizontal rule, correct?

@sneridagh
Copy link
Member

@stevepiercy yes, if possible.

Copy link
Member Author

@ksuess ksuess left a comment

Choose a reason for hiding this comment

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

@@ -148,10 +148,10 @@ Edit your {file}`package.json`:

```diff
"devDependencies": {
+ "@plone/scripts": ^3.0.0,
+ "@plone/scripts": "^3.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

Quotation marks are missing.

+ "@typescript-eslint/eslint-plugin": "6.7.0",
+ "@typescript-eslint/parser": "6.7.0",
+ "stylelint-prettier": "1.1.2",
+ "stylelint-prettier": "4.0.2",
Copy link
Member Author

Choose a reason for hiding this comment

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

"stylelint-prettier" version has been raised in the former diff to 4.x

@sneridagh
Copy link
Member

@stevepiercy I'm +1 to proceed from now on that way, pointing people to code instead of diffing in docs, let's do it for next contributions, ok? @ksuess PR is fine, as long as I can see and I'd merge it as it is.

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.

Typo and version mismatch in upgrade guide Volto 17
4 participants