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

Improve prefilled issue template #681

Merged
merged 5 commits into from
Sep 9, 2022

Conversation

Thomas1664
Copy link
Contributor

  • Fix markdown
  • Use Color::warning to better distinguish it from the rest of the error message
  • Fix printed path: The . after the path prevents users from navigating to the issue template directly from within the terminal.

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

Only nitpicks here. Thanks for your contribution!

LocalizedString create_user_troubleshooting_message(const InstallPlanAction& action,
const VcpkgPaths& paths,
Optional<Path>&& issue_body = nullopt);
LocalizedString create_user_troubleshooting_message(const InstallPlanAction& action, const VcpkgPaths& paths);
Copy link
Member

Choose a reason for hiding this comment

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

Since it's now an error to call this IMO it would be better to just inline it (and put the definition of print_user_troubleshooting_message into build.cpp?)

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 is not possible:

vcpkg-tool\src\vcpkg\install.cpp(525,17): error C3861: "print_user_troubleshooting_message": identifier not found

@BillyONeal BillyONeal merged commit 6b17755 into microsoft:main Sep 9, 2022
@BillyONeal
Copy link
Member

Thank you!

@Thomas1664 Thomas1664 deleted the issue-template branch September 10, 2022 13:24
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