Skip to content

Fix doc comments in Process.fs#2885

Open
Numpsy wants to merge 1 commit into
fsprojects:masterfrom
Numpsy:doc_comments
Open

Fix doc comments in Process.fs#2885
Numpsy wants to merge 1 commit into
fsprojects:masterfrom
Numpsy:doc_comments

Conversation

@Numpsy
Copy link
Copy Markdown
Contributor

@Numpsy Numpsy commented Nov 30, 2025

The Exec functions have a parameter called 'dir' but the doc comments have 'directory', so this just makes them consistent

(Just noticed a warning in Rider when looking at #2880 and though they should be consistent)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 30, 2025

Test Results

   12 files  + 2     12 suites  +2   43m 30s ⏱️ + 27m 15s
  452 tests ± 0    451 ✅ + 1  1 💤 ±0  0 ❌  - 1 
1 290 runs  +84  1 287 ✅ +85  3 💤 ±0  0 ❌  - 1 

Results for commit 6ab366b. ± Comparison against base commit e37773e.

♻️ This comment has been updated with latest results.

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 XML doc comments in Fake.Core.Process’s Shell API to match the actual optional parameter name (dir) and fixes a small grammar issue in the summary text.

Changes:

  • Update <param> name from directory to dir for Shell.Exec and Shell.AsyncExec.
  • Fix “it’s completion” → “its completion” in the Shell.Exec summary.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/app/Fake.Core.Process/Process.fs
Comment thread src/app/Fake.Core.Process/Process.fs
@xperiandri
Copy link
Copy Markdown
Collaborator

Are there any relevant comments from Copilot?

@Numpsy
Copy link
Copy Markdown
Contributor Author

Numpsy commented May 12, 2026

I wasn't looking at anything other than the param names before, but looking at the code now the 'async' function doesn't look very async? (actually it looks the same as the implementation of the not-async version)

@xperiandri
Copy link
Copy Markdown
Collaborator

Yep so I think we should get rid of that

@Numpsy
Copy link
Copy Markdown
Contributor Author

Numpsy commented May 13, 2026

Yep so I think we should get rid of that

It doesn't seem to be used from anywhere in this repo at least:

Numpsy@4c91022
https://github.com/Numpsy/FAKE/actions/runs/25744391229

So remove in v8 maybe

The Exec functions have a parameter called 'dir' but the doc comments have 'directory', so this just makes them consistent
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.

3 participants