Skip to content

update readme for slurm accounting changes#507

Merged
azreenz merged 1 commit into
masterfrom
azreenzaman/combined-certs
May 11, 2026
Merged

update readme for slurm accounting changes#507
azreenz merged 1 commit into
masterfrom
azreenzaman/combined-certs

Conversation

@azreenz
Copy link
Copy Markdown
Collaborator

@azreenz azreenz commented May 6, 2026

Update slurm accounting section in read me to account for new SSL certificate changes

Copilot AI review requested due to automatic review settings May 6, 2026 18:48
Copy link
Copy Markdown
Contributor

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

Updates the Slurm job accounting documentation in README.md to reflect the shift from supplying an SSL certificate URL to supplying a custom SSL certificate (CA bundle) content, aligning the README with the current template/UI behavior for accounting SSL configuration.

Changes:

  • Replace “SSL Certificate URL” guidance with “Custom SSL Certificate” guidance.
  • Document the default CA certificates used for Azure Database for MySQL Flex Server.
Criterion Max Points Points Awarded Notes
PR Description Accuracy (20) 20 0 PR description not provided (only a title was included).
PR Atomicity (20) 20 20 Single, focused documentation update.
Logical Implementation (10) 10 10 N/A (documentation-only) and straightforward.
Regression Risk (10) 10 10 No runtime/code behavior changes.
Exception Handling (10) 10 10 N/A (documentation-only).
Code Comments (10) 10 10 N/A (documentation-only).
Repetitive Code (10) 10 10 N/A.
Spelling (5) 5 5 No spelling issues identified in the changed lines.
Logging Quality (5) 5 5 N/A (documentation-only).

FINAL SCORE: 80/100

RECOMMENDATION: MERGE WITH FOLLOW-UPS
RATIONALE: Documentation is close to correct, but the current wording risks confusing users about whether they should paste URLs vs PEM contents (and one listed cert is DER-format). Address the noted doc clarification before merge; no code risk otherwise.
BLOCKERS:

  • Clarify in README that the field expects PEM contents (CA bundle), not URLs, and that the default Azure MySQL CA bundle is already provided unless the user overrides it.

Comment thread README.md Outdated
@azreenz azreenz force-pushed the azreenzaman/combined-certs branch 2 times, most recently from 831c807 to 59ba0f3 Compare May 6, 2026 19:02
@azreenz azreenz requested a review from Copilot May 6, 2026 19:03
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
@azreenz azreenz force-pushed the azreenzaman/combined-certs branch 3 times, most recently from dd65e98 to c83dfc0 Compare May 6, 2026 19:16
@azreenz azreenz requested a review from Copilot May 6, 2026 19:16
Copy link
Copy Markdown
Contributor

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

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

Comment thread README.md Outdated
@azreenz azreenz force-pushed the azreenzaman/combined-certs branch from c83dfc0 to 5bd5ebb Compare May 6, 2026 19:23
@azreenz azreenz merged commit 5e81456 into master May 11, 2026
3 checks passed
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.

4 participants