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

Restore VLT Training #880

Merged
merged 13 commits into from
Nov 23, 2024
Merged

Restore VLT Training #880

merged 13 commits into from
Nov 23, 2024

Conversation

stevepiercy
Copy link
Contributor

@stevepiercy stevepiercy commented Nov 18, 2024

Replaces #867, which had a strange merge.


📚 Documentation preview 📚: https://plone-training--880.org.readthedocs.build/

@stevepiercy
Copy link
Contributor Author

@danalvrz all right, I don't know what happened, so I refactored the directory name to match the training on https://2024.ploneconf.org/en/schedule/training/training-customizing-the-volto-light-theme, then pushed. Now the files are back and we have an open PR with a pull request preview build. I still have not reviewed it, as the naming confusing.

Replace curly quotes.
Revise index.md.
Flatten heading depth to promote concepts.
Terms are not inline literals.
Add scss as lexer for code blocks.
Spellcheck.
Fix heading levels.
Indent code blocks under their enumerated list items
Sentence-case heading.
Copy link
Contributor Author

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

There are serious flaws with the creating-new-project.md. See comments.

Trainer 101: Never publish something that you have not actually done yourself.

I want to keep rolling with my review, but this is definitely a blocker to publication.

@stevepiercy
Copy link
Contributor Author

@danalvrz I'm done with my review. There are critical issues with installation that must be fixed before this is merged. I strongly suggest not reinventing the wheel and just link to our docs, unless you have a Very Good Reason™ not to do so.

Copy link
Contributor Author

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

This looks good, with just one comment about a potential missing step to restart to see changes in another place.

@stevepiercy
Copy link
Contributor Author

@danalvrz I just sent you an invitation to give you access to merge this PR, once you've addressed my final comment. Please accept the invitation. Overall LGTM.

@danalvrz danalvrz merged commit 9df5b38 into main Nov 23, 2024
2 of 3 checks passed
@danalvrz danalvrz deleted the vlt-training branch November 23, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants