jinnanjiang commented on PR #767:
URL:
https://github.com/apache/shardingsphere-doc/pull/767#issuecomment-4539979569
Reviewed Scope
I reviewed the current patched state prepared for this PR, starting from
PR head 6496a57c74d2 and rechecking the full Slack-removal surface across:
- top-level site pages
- community/en/*
- affected blog/* pages
- affected document/*/overview/index.html pages
- elasticjob/*
- oncloud/*
- statistics/staging/rat-report.html
- affected English PDF artifacts under document/5.0.0, 5.1.0, 5.1.1, and
5.1.2
Not Reviewed Scope
- GitHub Actions / remote CI results
- deployed preview rendering in a browser
- unrelated site generation workflows beyond the artifacts touched here
Need Expert Review
- No
Root-Cause Review
The original problem was not just a few homepage links. The real issue was
that the site still exposed public Slack entrypoints through multiple
user-facing
paths, including:
- primary community onboarding pages
- generated feed content
- versioned documentation pages
- legacy artifacts
- published PDF artifacts
Earlier, this was only partially fixed. The remaining blockers were the
still-live Slack references in community/en/* and several residual artifacts.
That meant
users could still reach Slack through official participation pages even
after the main page cleanup.
Fix Mapping
The patched version now closes those remaining paths:
- community/en/involved/index.html no longer directs users to Slack
- community/en/involved/subscribe/index.html now points users only to
mailing-list based participation
- community/en/index.xml and community/en/involved/index.xml no longer
advertise Slack in feed descriptions
- community/en/team/index.html no longer tells contributors to join the
Slack community
- the remaining legacy overview pages that still exposed Slack have been
cleaned up
- the checked English PDF artifacts no longer contain embedded Slack URLs
- statistics/staging/rat-report.html no longer republishes Slack
references from the old content snapshot
This addresses the root cause rather than only removing a subset of
visible links.
Risk Scan
Design risk
- Low. This is a static content cleanup only.
- No build logic, navigation logic, templating logic, or runtime behavior
was changed.
Performance risk
- None. No execution path, asset-loading behavior, or client-side logic
changed.
Compatibility risk
- Low. Existing community entrypoints remain available through mailing
list, GitHub, and other public channels.
- No API, config, version-switcher, or URL routing behavior was changed.
Functional degradation risk
- Low. The change removes obsolete Slack references and preserves valid
alternative contact paths.
- I did not find any adjacent case where removing Slack changed unrelated
site behavior.
Cross-surface regression check
- I rechecked the broader blast radius instead of trusting the
homepage/blog diff alone.
- The residual-link scan covered the official community entrypoints,
generated feeds, legacy docs, and checked PDF artifacts.
Unrelated-Change Screening
- I did not find unrelated code or behavior changes in the patched result.
- The modified files are all directly tied to the Slack-removal objective.
Validation
I performed a fresh repository-wide residual scan for the known Slack URL
patterns:
- join.slack.com
- app.slack.com
- apacheshardingsphere.slack.com
In the reviewed website content and checked published artifacts, I did not
find any remaining public Slack URLs after the patch set. The remaining slack
text
hits are only icon/font class names, not user-facing community links.
Test Adequacy
- There are no automated tests for this static content cleanup.
- For this type of change, the relevant validation is artifact-level
verification and repository-wide residual scanning.
- That validation is sufficient here because the change is content-only
and the original failure mode was incomplete removal across generated/public
surfaces.
Merge Verdict: Mergeable
The patched version now removes the remaining public Slack entrypoints
across the reviewed site surfaces and artifacts, and I do not see unresolved
regression
or compatibility risk that would block merge.
Suggested PR comment
This looks good now. I rechecked the Slack-removal surface beyond the
homepage/blog/doc diffs, including the community/en/* entrypoints, feed
content, remaining
legacy overview pages, statistics/staging/rat-report.html, and the
affected English PDF artifacts. The remaining blockers from the previous pass
are now closed,
and I did not find any remaining public Slack URLs in the reviewed content.
Merge Verdict: Mergeable
--
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]
