Skip to content

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Feb 21, 2025

See discussion at #155 (comment)

This is on top #153 which has all the up to date PS1 scripts.


For the record, here's the prompts I gave.

  • update the documentation for the ps1 commands based on the rules
  • The PS1 scripts should not have a Bash shebang! Also, do it only for the scripts in bin/ the ones in tests/ are fine the way they are
  • mmm... why did you skip @path_aware_refreshenv.ps1 ?
  • thanks. revisit the requirements for @prepare_windows_host_for_app_distribution.ps1 are you sure about them?
    • at this point it added more requirements...
  • but... doesn't the script itself install Chocolately and jq?

I also asked it to generate the list of prompts I gave it, which it did but left out quite a few


At this point, I don't like this setup. The documentation it generates is very verbose. I suppose we could update the rules to make it more concise, removing redundant entries (like having requirements in notes and in the "Requirements" section). But I felt it took me as much time to get it to generate the docs and iterate on them, plus my final read, as it would have taken me to write a few lines explaining what the script does.

I'm not going to push against having this kind of documentation if @Automattic/apps-infrastructure feels it's useful. I acknowledge I have a low threshold for how much commented text I can read before looking at code. And after all, we did discuss doing a better job at documentation to help everyone in the team be productive across our tooling, particularly in the areas where one has less experience. But... I wonder if verbose headers docs are the way here vs smaller scripts or ad hoc comments to explain what each chunk does, for example.

I'm also okay to keep using LLMs to generate the docs. If we do this, I'd love to have a way to make it clear that's the primary author of the documentation. I feel as a reader it would help me understand why the docs read the way they do. And as a dev, it would make it clear that I should ask an LLM to generate the docs instead of — big air quotes — "waisting" time writing them myself from scartch.

mokagio and others added 30 commits February 14, 2025 17:35
Tested it by running on my Windows VM first this time.
@dangermattic
Copy link

dangermattic commented Feb 21, 2025

1 Warning
⚠️ Please add an entry in the CHANGELOG.md file to describe the changes made by this PR
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

AliSoftware and others added 5 commits February 21, 2025 16:28
* Introduce basic .cursorrules for this repo

* Add details about cross-platform considerations

And which commands we can assume are available in our agents for each type of script

* Add Script Validation section

* Improve .cursorrules documentation formatting and clarity

- Add backticks to code-like elements for better readability
- Standardize formatting of command and code references
- Enhance section formatting for improved visual consistency

* Add rule about shebang

* Reorder sections for consistency
* Add Cursor rules for PowerShell scripts

* Suggest Cursor to create a PowerShell script counterpart to bash scripts if it's not possible to make an original bash script compatible with Windows

* Add more context at the top of `.cursorrules`

* Migrate `.cursorrules` to `.cursor/rules/*.mdc`

* Add rule for using backticks around command names

* Add missing newline at EOF
It wrote them for the files in `tests/`, too, but I didn't want to
track those.
Cursor did not generate it the first time
It listed as requirements some of the things that the script installed
instead...
Comment on lines +4 to +5
# Installs the Windows 10 SDK and Visual Studio Build Tools based on a version
# specified in a .windows-10-sdk-version file.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Installs the Windows 10 SDK and Visual Studio Build Tools based on a version
# specified in a .windows-10-sdk-version file.
# Installs the Windows 10 SDK and Visual Studio Build Tools based on the
# version specified in a .windows-10-sdk-version file.

Comment on lines +22 to +23
# - Saves PATH before refreshenv and restores unique entries after
# - Use after installing packages that modify PATH at runtime
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems redundant

Suggested change
# - Saves PATH before refreshenv and restores unique entries after
# - Use after installing packages that modify PATH at runtime

# prepare_windows_host_for_app_distribution.ps1 -SkipWindows10SDKInstallation
#
# Notes:
# - Requires administrator privileges
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant because already in the requirements section

Suggested change
# - Requires administrator privileges

Comment on lines +23 to +26
# Notes:
# - Requires .windows-10-sdk-version file in current directory
# - Version must be one of the allowed SDK versions from Visual Studio
# - Installation requires admin privileges
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a second pass of the diff, I noticed that none of the docs used the full spec from .cursor/rules/powershell_scripts.mdc

.NOTES
    Author: [Author name]
    Last Edit: [Date]
    Version 1.0 - Initial release
    Requirements: Windows PowerShell 5.1
    Windows Requirements:
        - Windows Server 2019 or later
        - Required Windows Features: [list features]
        - Required Windows Roles: [list roles]
        - Administrator privileges: [Yes/No]

Maybe I didn't prompt it well enough?

@AliSoftware
Copy link
Contributor

@mokagio I did the same experiment on my end asking my Cursor Composer to generate those docs too, you can see the result in #160. I left the generated content as-is in the draft PR to highlight what it generated without any manual tweaking.

PS: Did you close the folder/window and quit Cursor before switching to the commit that contained the .mdc files then reopening Cursor only then, before trying to ask it for the prompt?

@mokagio mokagio mentioned this pull request Feb 24, 2025
1 task
@mokagio
Copy link
Contributor Author

mokagio commented Feb 24, 2025

Did you close the folder/window and quit Cursor before switching to the commit that contained the .mdc files then reopening Cursor only then, before trying to ask it for the prompt?

I didn't record myself going through the process so I might be mistaken, but I'm pretty sure I did because I remember reading about that gotcha either in the other docs PR or on Slack.

@mokagio
Copy link
Contributor Author

mokagio commented Feb 24, 2025

I left the generated content as-is in the draft PR to highlight what it generated without any manual tweaking.

This comment actually makes me think of something interesting / noteworthy / that should be kept in mind when thinking of LLM-generated docs: The result is non-deterministic!

I don't think that's a show stopper, and I'd like to think that with a set of rules well configured, the result should be similar across different prompt styles. But that's something I don't think I ever thought about and that makes me a bit uncomfortable, in a sense. In a world where we go through a lot of trouble to get deterministic result (VMs, lock-files, inject app state to decouple unit tests from it, etc.) getting different results every time is unsettling.

Again, I don't think it's a blocker, especially when it comes to documentation written in natural language and with the expectation that the better we get at configuring the .cursor/rules and at prompting, the better and more reliable the results will be.

@AliSoftware
Copy link
Contributor

That is indeed good to keep in mind. Though, to be clear, the use of LLM here was never intended to generate (deterministic) documentation.

After all, the goal is not to have the code not documented and have a process that would generate documentation dynamically on the fly each time we need it, but rather to have a robot intern help us write some documentation for code so we can review and commit it once it's written. In that sense, the LLM is just like a robot intern. We might each have a slightly different intern each with its slightly different voice and tone and style of writing, but that doesn't really matter if the writing style is different between each attempt at asking the LLM the same question from different machines or at different times, what matters is that we end up with some sort of documentation that makes sense and is helpful.

Then the more documentation we'll write as headers of our shell scripts then commit to git, the more likely it will be, when the next person asking their LLM to write new documentation for another script, to pick us a similar tone to the documentation that was already written, as it'd likely use it as inspiration. But even if it didn't what matters is that the LLM is only here as an intern to help speed up the process and gather the information to write an initial take. It doesn't have to be deterministic.

Base automatically changed from mokagio/windows-10-experiments to mokagio/more-windows-utils February 26, 2025 00:51
Base automatically changed from mokagio/more-windows-utils to trunk February 28, 2025 02:43
mokagio added a commit that referenced this pull request Feb 28, 2025
I had already set this up in #159, but we'll likely close that PR
without merging it, so here's a standalone one.
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