Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update README.md PowerShell Script #54

Merged
merged 2 commits into from
Dec 13, 2022

Conversation

TheReptar
Copy link
Contributor

@TheReptar TheReptar commented Nov 29, 2022

Context:

  • Per "failed to get console mode for stdout" in windows powershell #30, the README.md script provided for Windows/PowerShell users fails to navigate to the selected directory.
    • The issue appears to be related to an upstream dependency.
  • Onboarding users should be able to copy/paste the provided script and have it work as expected.

Changes (Functional):

  • None.

Additional Notes & Tech Debt:


@TheReptar TheReptar changed the title Update readme powershell script Update README.md PowerShell Script Nov 29, 2022
@TheReptar
Copy link
Contributor Author

@antonmedv This PR has been sitting for a couple weeks; are there any modifications you're waiting on that are preventing this from being merged?

@antonmedv
Copy link
Owner

Let’s fix upstream issue.

@TheReptar
Copy link
Contributor Author

What if we remove the association with #30 from this PR and merge this documentation change (that way the issue won't be closed when this is merged). Progress on the upstream issues is slow, and I'd like the README to provide a Powershell script that works in the meantime -- the currently provided script will fail for new users.

@antonmedv
Copy link
Owner

Yes, lets do it

@TheReptar
Copy link
Contributor Author

Okay, I've removed the closing association 👍

}
```
See [issues/30](https://github.com/antonmedv/llama/issues/30) for more details.
Copy link
Owner

Choose a reason for hiding this comment

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

Still in pr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I kept that in the actual README so that when users read the setup docs they can understand that the script is actually a workaround for an upstream issue (all that info is in #30), that's something that could be removed once the issue is closed and the script is updated again. I just removed the association in the PR text that would close the issue once it's merged.

@antonmedv antonmedv merged commit 130e6c2 into antonmedv:master Dec 13, 2022
@TheReptar TheReptar deleted the update-readme-powershell-script branch December 14, 2022 15:05
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.

2 participants