docs: improve enterprise license docs - #2654
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Correct the command-substitution quoting and clarify filename versus PEM-content requirements.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Improves enterprise license installation documentation for Timefold Solver.
Changes:
- Clarifies PEM filename and certificate requirements.
- Adds a
TIMEFOLD_LICENSEenvironment-variable example. - Simplifies installation guidance.
File summaries
| File | Description |
|---|---|
docs/src/modules/ROOT/pages/commercial-editions/installation.adoc |
Updates enterprise license setup instructions. |
Review details
Suppressed comments (1)
docs/src/modules/ROOT/pages/commercial-editions/installation.adoc:31
- This sentence makes the certificate marker lines sound like part of the filename-matching requirement, which ambiguously tells users what must be present in the PEM file; separate the filename and file-content requirements.
Take care that the file name matches exactly, including letter case and the lines containing "BEGIN CERTIFICATE" and "END CERTIFICATE".
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
The license-loading example and requirement wording need correction before approval.
Review details
Suppressed comments (2)
docs/src/modules/ROOT/pages/commercial-editions/installation.adoc:34
- Because the command substitution is enclosed in single quotes, the shell stores the literal text
$(cat ./path/to/timefold-license.pem)instead of the PEM contents; use double quotes around the command substitution so the example actually loads the license file.
* Store the license's PEM string (the contents of the license file) in the `TIMEFOLD_LICENSE` environment variable, e.g. with the `export TIMEFOLD_LICENSE='$(cat ./path/to/timefold-license.pem)'`.
docs/src/modules/ROOT/pages/commercial-editions/installation.adoc:31
- This sentence conflates the filename requirement with the PEM-content requirement, making it read as though the filename must include the certificate marker lines; separate the exact filename rule from the requirement that the file contents contain those markers.
Take care that the file name matches exactly, including letter case and the lines containing "BEGIN CERTIFICATE" and "END CERTIFICATE".
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
triceo
left a comment
triceo
left a comment
There was a problem hiding this comment.
Good to go when no open comments.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Clarify the distinction between the required filename and PEM-format markers.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
docs/src/modules/ROOT/pages/commercial-editions/installation.adoc:31
- This note grammatically makes the filename “match” the PEM marker lines, so readers cannot tell whether the markers are required in the file contents or part of the filename; separate the filename requirement from the PEM-format requirement.
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
No description provided.