-
Notifications
You must be signed in to change notification settings - Fork 296
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
Add new post check for share folder #675
base: main
Are you sure you want to change the base?
Add new post check for share folder #675
Conversation
…ev/jack/new-lint-postbuild-check
@ras0219-msft For regression, should I split the message to fix it? |
* Move FolderNameMismatchedCasing to messages.h / messages.cpp * Remove trailing \n * Regenerate jsons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the build failures and I think the change is overall good. Because it adds a new post-build lint we need to pre-run it on the catalog to double check that the world won't break when deployed. @ras0219-msft Can you confirm you're OK with this now and I can kick that off?
The error was complaining that the localized message ended with |
LGTM. @BillyONeal if you could kick off the test that would be great! |
Well, you can just use the file lists from CI... I find
|
Why folders in the share folder should be always lowercase? |
Good point!
The name vcpkg creates is always, for example, |
Example we want the new check to block microsoft/vcpkg#26284 (comment) |
Port ogre installs resources to |
This mirrors the headers in |
…ev/jack/new-lint-postbuild-check # Conflicts: # include/vcpkg/base/messages.h # locales/messages.en.json # locales/messages.json # src/vcpkg/base/messages.cpp
That's a great point. We do need a policy setting to control the check. Then, we should evaluate whether ogre should disable the policy or whether patching is appropriate. I don't think that matching |
@JackBoosY Once again, thank you for all your hard work on vcpkg! Would you like to continue this PR or should we take ownership and bring it to completion? |
@ras0219-msft I will continue this PR. |
…ev/jack/new-lint-postbuild-check
Looks good now. |
It still needs a policy to disable it, and changes to app. 60 ports. |
@ras0219-msft @BillyONeal Any feedback? |
The feedback is only four days old (#675 (comment))
|
Ah I'm blind... |
…ev/jack/new-lint-postbuild-check
So what about the 60+ ports / port OGRE? |
FTR at least to open PRs add new ports with mixed case directories via |
Will do that after microsoft/vcpkg#29880 microsoft/vcpkg#29882 microsoft/vcpkg#29883 microsoft/vcpkg#29884 and microsoft/vcpkg#29918 are merged. |
…ev/jack/new-lint-postbuild-check
Depends on microsoft/vcpkg#31053 |
Check for non-lowercase share folder.