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

Remove keywords 'or' in Platform Expression #90

Merged
merged 3 commits into from
Jul 17, 2023

Conversation

LilyWangLL
Copy link
Contributor

Keywords or is invalid, this change from microsoft/vcpkg-tool#267, remove it from vcpkg-json.md.

https://github.com/microsoft/vcpkg-tool/blob/2e31d678b42aa56c3a44083d431829158814ded6/src/vcpkg-test/platform-expression.cpp#LL506C28-L506C28

invalid logic expression, use '|' instead of 'or'

Fixes microsoft/vcpkg#31683

@prmerger-automator
Copy link

@LilyWangLL : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit 6167ac9:

✅ Validation status: passed

File Status Preview URL Details
vcpkg/reference/vcpkg-json.md ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@dg0yt
Copy link
Contributor

dg0yt commented May 29, 2023

I would assume that the documentation for not and and needs to be fixed, too.

@LilyWangLL
Copy link
Contributor Author

I would assume that the documentation for not and and needs to be fixed, too.

I tested not and and and they work fine.

@dg0yt
Copy link
Contributor

dg0yt commented May 29, 2023

Then consistency would imply that or works, too. Principle of least surprise.

@autoantwort
Copy link
Contributor

Never understand why microsoft/vcpkg-tool#267 was merged. Afaik it is used by no one and added a second syntax for the same thing.

@LilyWangLL
Copy link
Contributor Author

cc @@markle11m cc @vicroms

@vicroms
Copy link
Member

vicroms commented Jun 26, 2023

@autoantwort this was part of the ce (artifacts) merger, we had to support their platform expressions which were implemented using media queries.

@LilyWangLL we should add a text explaining that while not accepted or is a reserved keyword.

@vicroms vicroms marked this pull request as draft June 26, 2023 22:02
Add text explaining
@LilyWangLL LilyWangLL marked this pull request as ready for review June 27, 2023 10:28
@LilyWangLL LilyWangLL requested a review from vicroms June 27, 2023 10:28
@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit 49cb01a:

✅ Validation status: passed

File Status Preview URL Details
vcpkg/reference/vcpkg-json.md ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@vicroms
Copy link
Member

vicroms commented Jul 10, 2023

@LilyWangLL could you apply this patch to fix some lint issues in this article.

@learn-build-service-prod
Copy link
Contributor

Learn Build status updates of commit 3ad61ab:

✅ Validation status: passed

File Status Preview URL Details
vcpkg/reference/vcpkg-json.md ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@LilyWangLL
Copy link
Contributor Author

@LilyWangLL could you apply this patch to fix some lint issues in this article.

Done, thanks for your help.

@vicroms vicroms merged commit b1ab726 into microsoft:main Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error: invalid logic expression, use '|' instead of 'or'
5 participants