Skip to content

Conversation

@neriyaco
Copy link
Member

No description provided.

@neriyaco neriyaco requested review from binyamin555 and Copilot March 29, 2025 23:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR for "Pydom v0.4" updates the documentation and various GitHub workflows to integrate a new middleware and improve release processes. Key changes include:

  • Updating README.md to reference a new script URL and middleware (SocketIOMiddleware) for server integration.
  • Enhancing documentation build configuration in .readthedocs.yaml and broadening the test matrix in python-test.yml.
  • Modifying release and node package workflows with added permissions, branch creation, and a shift from a build script to a publish script.

Reviewed Changes

Copilot reviewed 121 out of 136 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
README.md Updated script URL and middleware import/use for improved clarity.
.readthedocs.yaml Added an alternative method for package installation in docs.
.github/workflows/python-test.yml Expanded supported Python versions for testing.
.github/workflows/create-release.yaml Introduced additional permissions, environment variable, and branch creation in the release workflow.
.github/workflows/build-node-packages.yml Changed from build.js to publish.js for node package releases.
Files not reviewed (15)
  • .vscode/settings.json: Language not supported
  • docs/.gitignore: Language not supported
  • docs/3-events/1-middleware.rst: Language not supported
  • docs/99-api-reference/1-components/1-base.rst: Language not supported
  • docs/99-api-reference/1-components/index.rst: Language not supported
  • docs/99-api-reference/1-components/page.rst: Language not supported
  • docs/99-api-reference/1-components/router/index.rst: Language not supported
  • docs/99-api-reference/1-components/router/route.rst: Language not supported
  • docs/99-api-reference/1-components/router/router-link.rst: Language not supported
  • docs/99-api-reference/1-components/router/router.rst: Language not supported
  • docs/99-api-reference/2-state/index.rst: Language not supported
  • docs/99-api-reference/3-core/index.rst: Language not supported
  • docs/99-api-reference/3-core/rendering.rst: Language not supported
  • docs/99-api-reference/99-misc/index.rst: Language not supported
  • docs/99-api-reference/index.rst: Language not supported
Comments suppressed due to low confidence (2)

.github/workflows/build-node-packages.yml:32

  • Ensure that the 'publish.js' script correctly handles the release tag as expected, since its behavior might differ from the previous 'build.js'.
node publish.js ${{ github.event.release.tag_name }}

.github/workflows/create-release.yaml:16

  • Verify that using the GitHub token in this context complies with your security policies and provides the intended permissions without exposing sensitive data.
GH_TOKEN: ${{ github.token }}

run: |
git config --local user.email "${{ github.actor }}@users.noreply.github.com"
git config --local user.name "${{ github.actor }}"
git checkout -b release/$VERSION
Copy link

Copilot AI Mar 29, 2025

Choose a reason for hiding this comment

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

The branch name 'release/$VERSION' may be problematic if VERSION contains characters not allowed in git branch names. Consider sanitizing or validating the VERSION variable before using it.

Copilot uses AI. Check for mistakes.
@neriyaco
Copy link
Member Author

I see there's a lot of work to do with the documentation, so we're not going to merge this yet.

@neriyaco neriyaco marked this pull request as draft March 29, 2025 23:42
@binyamin555
Copy link
Member

There's also a lot of work to review this, so don't merge it yet

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