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

DOC-2347 Automation script for generating new plugin docs #3181

Closed
wants to merge 18 commits into from

Conversation

FarzadHayat
Copy link
Contributor

@FarzadHayat FarzadHayat commented Mar 25, 2024

Ticket: DOC-2347

Changes:

  • Add script for generating new plugin docs
  • Clean-up of templates files

Pre-checks:

  • Branch prefixed with feature/7/ or hotfix/7/
  • Files has been included where required (if applicable)

Review:

  • Documentation Team Lead has reviewed

@FarzadHayat FarzadHayat self-assigned this Mar 25, 2024
@FarzadHayat FarzadHayat requested review from kemister85 and removed request for kemister85 March 25, 2024 07:14
@FarzadHayat FarzadHayat marked this pull request as ready for review April 2, 2024 07:28
@FarzadHayat FarzadHayat requested a review from a team as a code owner April 2, 2024 07:28
@FarzadHayat FarzadHayat requested review from Afraithe, EkimChau, ShiridiGandham, kemister85 and a team and removed request for a team, Afraithe and EkimChau April 2, 2024 07:28
@FarzadHayat FarzadHayat requested review from MitchC1999 and removed request for a team April 2, 2024 07:29
Copy link
Contributor

@kemister85 kemister85 left a comment

Choose a reason for hiding this comment

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

  • @FarzadHayat - please do not merge this until I get a chance to review and test please.

Copy link
Member

@TheSpyder TheSpyder left a comment

Choose a reason for hiding this comment

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

I suspect those template names for folders and files will not work on a windows machine. Not all of our developers are on mac/linux.

This isn't how I would solve the problem, but if the docs team is comfortable with it I won't stand in your way.

@FarzadHayat
Copy link
Contributor Author

FarzadHayat commented Apr 9, 2024

@TheSpyder good point, I had not considered windows compatibility when writing this script.

While looking into this, I discovered that tinymce-docs default branch is not even compatible with windows because some template files in the -new-material-templatesdirectory use < and > in the name which results in errors when git cloning the repo on windows. I was able to get around this by replacing < and > with _ and that fixed the git cloning issue. A temporary workaround for windows is to run the script using git bash with only some minor tweaks to the script.

Do you have any suggestions for how you would approach solving this problem?

@TheSpyder
Copy link
Member

TheSpyder commented May 7, 2024

@FarzadHayat sorry, I don't always see @ pings as I get about 100 github emails a week and tend to rely on review requests.

Apparently I wasn't paying attention to the filenames when I reviewed #2801!

For windows, I don't mind saying WSL is required (we do that on some of our other repositories). But that only solves the bash issue, not the filename issue.

When I said This isn't how I would solve the problem, what I meant was not having template files in the repo and copying them with scripts. I'd look at yeoman or another similar tool that is designed to take a template structure and fill in the gaps to generate a series of files.

Focus on the template content, which is the real concern here, not the act of using templates for which there are many options available and isn't our core competency.

@TheSpyder TheSpyder marked this pull request as draft May 8, 2024 00:42
@TheSpyder
Copy link
Member

Marked as draft to stop it notifying the team until the questions are sorted out

@FarzadHayat
Copy link
Contributor Author

FarzadHayat commented Jun 21, 2024

This ticket has been split into multiple tickets (DOC-2469 and DOC-2470) which must be completed before this ticket.
The target branch for this PR is now archived, so I am closing this PR. Will branch out in the future using the new tinymce/7 branch when ready for development. Keep the branch alive for future reference.

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.

3 participants