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

Document Connectors #1518

Merged
merged 3 commits into from
Jul 25, 2024
Merged

Document Connectors #1518

merged 3 commits into from
Jul 25, 2024

Conversation

tharsheblows
Copy link
Contributor

Fixes #1515

Adds a cheap and cheerful script that runs locally to create the Connectors documentation and also adds the documentation itself.

Checklist

  • Project documentation has been updated to reflect the changes in this pull request, if applicable.
  • I have tested the changes in the local development environment (see contributing.md).
  • I have added phpunit tests.

Cheap and cheerful, runs off your local php, may not work for everyone. Let me know what issues you have!
Copy link
Contributor

@delawski delawski left a comment

Choose a reason for hiding this comment

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

This is fantastic, @tharsheblows! 🤩 I left a thought but it is ready to be merged as is 👍

@@ -57,6 +57,7 @@
"switch-to:php8.2": "docker compose build --build-arg PHP_VERSION=8.2 --build-arg XDEBUG_VERSION=3.3.2 && npm run start",
"switch-to:php7.4": "docker compose build && npm run start",
"which-php": "npm run cli -- php --version",
"document:connectors": "php ./local/scripts/generate-connector-docs.php",
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 thought: I was wondering if it'd be worth running this script from within the Docker container. Just so that we do not rely on the developer's local environment setup. Also, I guess by running it through Docker, we could make this job part of a GitHub action in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@delawski It's not copied to the container so we'd need to pipe it in and I can only figure out how to do it via docker exec ... which requires the container name or id which can change a bit (eg mine is stream-wordpress-1 because I already had a stream-wordpress I guess.

Or we could copy it into the container but ... I don't know that seems a bit much?

@delawski delawski added this to the 4.0.1 milestone Jul 25, 2024
@tharsheblows tharsheblows requested a review from delawski July 25, 2024 14:27
@tharsheblows tharsheblows merged commit 74aeca6 into develop Jul 25, 2024
2 checks passed
@tharsheblows tharsheblows deleted the docs/connectors-script branch July 25, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document current Connectors
2 participants