Copilot commented on code in PR #414:
URL: https://github.com/apache/fluss-rust/pull/414#discussion_r2875738129
##########
website/static/manifest.json:
##########
@@ -0,0 +1,17 @@
+{
+ "short_name": "Fluss Clients",
+ "name": "Apache Fluss Clients: Rust, Python, and C++",
+ "description": "Rust, Python, and C++ clients for Apache Fluss",
+ "start_url": "/",
+ "scope": "/",
+ "display": "standalone",
+ "background_color": "#000000",
+ "theme_color": "#0071e3",
+ "icons": [
+ {
+ "src": "img/logo/svg/colored_logo.svg",
+ "sizes": "any",
+ "type": "image/svg+xml"
Review Comment:
The manifest only provides an SVG icon (`sizes: any`). Some PWA install
flows expect raster icons (commonly 192x192 and 512x512 PNGs) and may show a
generic icon otherwise. Consider adding PNG icons with explicit sizes (you
already have `website/static/img/logo/png/colored_logo.png`) and listing them
alongside the SVG.
```suggestion
"type": "image/svg+xml"
},
{
"src": "img/logo/png/colored_logo.png",
"sizes": "192x192",
"type": "image/png"
},
{
"src": "img/logo/png/colored_logo.png",
"sizes": "512x512",
"type": "image/png"
```
##########
.github/workflows/deploy_documentation.yml:
##########
@@ -0,0 +1,80 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: Deploy Documentation
+
+on:
+ workflow_dispatch:
+
+permissions:
+ contents: write
+
+jobs:
+ deploy:
+ runs-on: ubuntu-latest
+ defaults:
+ run:
+ working-directory: ./website
+ steps:
+ - uses: actions/checkout@v6
+ with:
+ fetch-depth: 0
+
+ - uses: actions/setup-node@v6
Review Comment:
The workflow uses `actions/checkout@v6` and `actions/setup-node@v6`, while
most other workflows in this repo pin `actions/checkout@v4` (e.g.,
`.github/workflows/build_and_test_rust.yml`). To reduce risk of inconsistent
behavior (and potential resolution failures if the major version tag isn’t
available), consider aligning these action versions with the rest of the repo.
```suggestion
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-node@v4
```
##########
website/docusaurus.config.ts:
##########
@@ -20,6 +20,25 @@ const config: Config = {
locales: ['en'],
},
+ plugins: [
+ [
+ '@docusaurus/plugin-pwa',
+ {
+ debug: true,
+ offlineModeActivationStrategies: [
+ 'appInstalled',
+ 'standalone',
+ 'queryString',
+ ],
Review Comment:
`@docusaurus/plugin-pwa` is configured with `debug: true`, which is
typically meant for local debugging and can add noisy logging / dev behavior in
production deployments. Consider making this conditional (e.g., based on
`NODE_ENV`) or defaulting it to `false` for production builds.
##########
.github/workflows/deploy_documentation.yml:
##########
@@ -0,0 +1,80 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: Deploy Documentation
+
+on:
+ workflow_dispatch:
+
+permissions:
+ contents: write
+
+jobs:
+ deploy:
+ runs-on: ubuntu-latest
+ defaults:
+ run:
+ working-directory: ./website
+ steps:
+ - uses: actions/checkout@v6
+ with:
+ fetch-depth: 0
+
+ - uses: actions/setup-node@v6
+ with:
+ node-version: 24
+
+ - name: Install dependencies
+ run: npm install
+
+ - name: Build website
+ run: npm run build
+
+ - name: Deploy to gh-pages branch
+ working-directory: .
+ env:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: |
+ git config user.name "github-actions[bot]"
+ git config user.email "github-actions[bot]@users.noreply.github.com"
+
+ # Create a temporary directory with the built site
+ TMPDIR=$(mktemp -d)
+ cp -r website/build/* "$TMPDIR"
+
+ # Switch to the gh-pages branch (create orphan if it doesn't exist)
+ if git ls-remote --exit-code origin gh-pages; then
+ git fetch origin gh-pages
+ git checkout gh-pages
+ else
+ git checkout --orphan gh-pages
+ git rm -rf .
+ fi
+
+ # Replace contents with the new build
+ git rm -rf . || true
+ cp -r "$TMPDIR"/* .
+ rm -rf "$TMPDIR"
+
+ # Commit and push
+ git add -A
+ if git diff --cached --quiet; then
Review Comment:
The deploy script can accidentally commit CI build artifacts (e.g.,
`website/node_modules`, `website/build`) to `gh-pages`. Those directories are
currently ignored on `main`, but after `git rm -rf .` runs on `gh-pages`,
`.gitignore` is removed, so any leftover untracked files from the earlier `npm
install`/`npm run build` can become stageable and get included by `git add -A`.
Clean the working tree (e.g., `git clean -fdx` / remove the `website/` dir)
before deleting `.gitignore` and staging the deploy content, or build outside
the repo checkout entirely.
##########
.github/workflows/deploy_documentation.yml:
##########
@@ -0,0 +1,80 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: Deploy Documentation
+
+on:
+ workflow_dispatch:
Review Comment:
This workflow only runs on `workflow_dispatch`. If the intent is to “publish
docs” automatically when `website/**` changes on `main` (as implied by the
linked issue), consider adding a `push` trigger (optionally filtered by `paths:
['website/**']`) so deployments happen without manual intervention.
```suggestion
workflow_dispatch:
push:
branches:
- main
paths:
- 'website/**'
```
##########
website/package.json:
##########
@@ -12,6 +12,7 @@
},
"dependencies": {
"@docusaurus/core": "^3.9.2",
+ "@docusaurus/plugin-pwa": "^3.9.2",
"@docusaurus/preset-classic": "^3.9.2",
Review Comment:
PR description focuses on setting up a docs publish workflow, but this
change also introduces PWA support (plugin + manifest) and changes the site
`url/baseUrl` to a new custom domain. If those are intentional for issue #401,
consider updating the PR description to reflect the broader scope; otherwise,
consider splitting these into a separate PR to keep deploy/workflow changes
isolated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
