Skip to content

docs: update README for documentation freshness#639

Open
dlevy-msft-sql wants to merge 10 commits into
microsoft:mainfrom
dlevy-msft-sql:docs/fix-readme-freshness
Open

docs: update README for documentation freshness#639
dlevy-msft-sql wants to merge 10 commits into
microsoft:mainfrom
dlevy-msft-sql:docs/fix-readme-freshness

Conversation

@dlevy-msft-sql
Copy link
Copy Markdown
Contributor

@dlevy-msft-sql dlevy-msft-sql commented Jan 26, 2026

Summary

Updates the README.md to fix documentation freshness issues and improve accuracy.

Changes

Fixed Issues

  1. Discussion link typo: Fixed link text from Require --password-encryption for add-user #293 to Prioritize implementation of back-compat flags #292 (the link URL was correct but display text was wrong)

  2. Updated 'Switches not available' section: Reflects current implementation state

    • Most switches from the original discussion have now been implemented: -e, -g, -k, -t, -z, -Z, -r, -X
    • -f (input/output code page), -j (raw error messages), and -p[1] (print statistics) remain unimplemented
    • Changed from vague text to a clear table of remaining items
  3. Rewrote ActiveDirectoryIntegrated description: The previous text just said "currently not implemented and will fall back to ActiveDirectoryDefault". The fallback is still real (the go-mssqldb driver does not implement true Integrated auth), so the rewrite preserves that fact and explains why — the gap is in the driver, not in sqlcmd's wiring. No behavior change.

  4. Added missing authentication methods to :Connect -G list:

    • ActiveDirectoryInteractive
    • ActiveDirectoryAzCli
    • ActiveDirectoryDeviceCode
    • Also replaced SqlAuthentication (never a recognized value) with SqlPassword
  5. Documented additional authentication methods that are available via --authentication-method:

    • ActiveDirectoryWorkloadIdentity
    • ActiveDirectoryClientAssertion
    • ActiveDirectoryAzurePipelines
    • ActiveDirectoryEnvironment
    • ActiveDirectoryAzureDeveloperCli
    • ActiveDirectoryServicePrincipalAccessToken
    • SqlPassword

    For WorkloadIdentity and AzurePipelines, the docs only mention the supported environment variables and explicitly note that sqlcmd has no mechanism to pass per-connection parameters like tokenfilepath, serviceconnectionid, or systemtoken.

Related

This addresses documentation freshness issues identified when comparing the README with the actual implementation in cmd/sqlcmd/sqlcmd.go, pkg/sqlcmd/azure_auth.go, pkg/sqlcmd/connect.go, and the Microsoft Learn documentation.

Depends on

Testing

Documentation-only change - no functional changes.

@dlevy-msft-sql dlevy-msft-sql self-assigned this Jan 27, 2026
@dlevy-msft-sql dlevy-msft-sql added documentation Improvements or additions to documentation Size: S Small issue (less than one week effort) labels Jan 27, 2026
Copy link
Copy Markdown

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 updates the README.md to improve documentation freshness and accuracy by correcting outdated information and adding missing details.

Changes:

  • Fixed discussion link typo from #293 to #292
  • Updated "Switches not available" section to reflect current implementation (most switches now implemented, only -j and -p[1] remain)
  • Corrected ActiveDirectoryIntegrated authentication description (removed outdated fallback text)
  • Added missing authentication methods to documentation (ActiveDirectoryInteractive, ActiveDirectoryAzCli, ActiveDirectoryDeviceCode, and additional methods via --authentication-method)

Copy link
Copy Markdown

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 1 changed files in this pull request and generated 4 comments.

Comment thread README.md Outdated
Comment thread README.md
Comment thread README.md Outdated
Comment thread README.md Outdated
- Fix discussion link typo (microsoft#293 -> microsoft#292)
- Update 'Switches not available' section to reflect current state
  - Most switches now implemented (-e, -f, -g, -k, -t, -z, -Z, -r, -X)
  - Only -j and -p[1] remain unimplemented
- Add missing authentication methods to :Connect documentation
  - ActiveDirectoryInteractive, ActiveDirectoryAzCli, ActiveDirectoryDeviceCode
- Fix ActiveDirectoryIntegrated description (no longer falls back)
- Document additional authentication methods:
  - ActiveDirectoryWorkloadIdentity
  - ActiveDirectoryClientAssertion
  - ActiveDirectoryAzurePipelines
  - ActiveDirectoryEnvironment
  - ActiveDirectoryAzureDeveloperCli
  - ActiveDirectoryServicePrincipalAccessToken
  - SqlPassword
…e auth docs

- Revert ActiveDirectoryIntegrated to accurate fallback description
  (driver still falls back to DefaultAzureCredential per source comment)
- Add -f (codepage) to unimplemented switches table
- Replace generic auth method descriptions with driver-verified details
  including actual env var names and connection parameters
- Expand -p[1] from 'optional colon format' to explain what it does
- Explain tokenfilepath is a path to the federated token file
- Add ClientAssertion usage: client_id@tenant_id as user, JWT as password
- Replace SqlAuthentication with SqlPassword in :Connect -G list
  (SqlAuthentication is not a recognized value; -G routes unknown
  strings to the AAD path)
- Note that ServicePrincipalAccessToken does not currently propagate
  SQLCMDPASSWORD into the connection string
- Remove mention of tokenfilepath/serviceconnectionid/systemtoken as
  connection parameters; sqlcmd has no way to pass them, only env vars
@dlevy-msft-sql dlevy-msft-sql force-pushed the docs/fix-readme-freshness branch from 86073c5 to e3c27b7 Compare May 19, 2026 20:27
@dlevy-msft-sql dlevy-msft-sql requested a review from Copilot May 19, 2026 20:30
Copy link
Copy Markdown

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 1 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
- Fix double space in :Connect -G description (line 178)
- Soften ActiveDirectoryServicePrincipalAccessToken wording and link tracking issue microsoft#758
- -f tracked by microsoft#628 (and microsoft#111)
- -j and -p[1] still only in discussion microsoft#292
- -p[1] tracked by open PR microsoft#631
- -j: only a closed/unmerged attempt (microsoft#624); kept discussion microsoft#292 as primary ref
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation Priority: 1 High priority/impact Size: S Small issue (less than one week effort)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants