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.

Sign up for GitHub

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

Jump to bottom

replacing applocal.ps1 with C++ 😍 #814

Merged
merged 58 commits into from
Dec 31, 2022
Merged

replacing applocal.ps1 with C++ 😍 #814

merged 58 commits into from
Dec 31, 2022

Conversation

valeriaconde
Copy link
Contributor

@valeriaconde valeriaconde commented Nov 18, 2022 •
edited
Loading

Please review 🙏

@valeriaconde valeriaconde marked this pull request as ready for review November 30, 2022 17:23
BillyONeal
BillyONeal reviewed Dec 28, 2022
View reviewed changes
BillyONeal
BillyONeal approved these changes Dec 28, 2022
View reviewed changes
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 left :)

@BillyONeal BillyONeal requested a review from JackBoosY December 28, 2022 21:37
vicroms
vicroms approved these changes Dec 28, 2022
View reviewed changes
Copy link
Member

@vicroms vicroms left a comment

Choose a reason for hiding this comment

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

LGTM. I'd like to have continuous tests for the "library specific" ones, but they worked when tested locally.

BillyONeal
BillyONeal approved these changes Dec 29, 2022
View reviewed changes
@dg0yt
Copy link
Contributor

dg0yt commented Dec 30, 2022

This PR hard codes informations about some plugin binaries for some ports into a binary which is maintained in another repository than the ports which provide these plugins.

  • How is this meant to be kept in sync with the actual set of plugin binaries which may vary with the port versions and features?
  • How is this meant to scale to the ports which provide plugins but aren't handled yet, e.g. osg/osgearth?

@vicroms
Copy link
Member

vicroms commented Dec 30, 2022

@dg0yt

This is Valeria's internship project; it is intended to be phase 1 of removing our dependence on PowerShell. We intend to design and implement a generic approach after merging this PR.

ras0219-msft
ras0219-msft approved these changes Dec 30, 2022
View reviewed changes
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

LGTM if we add deployment of both vcpkg.exe AND applocal.ps1 into export.

@ras0219-msft ras0219-msft merged commit e8c0b20 into microsoft:main Dec 31, 2022
@ras0219-msft
Copy link
Contributor

Thanks Val!

@vicroms vicroms mentioned this pull request Jan 7, 2023
Merged
@BillyONeal BillyONeal mentioned this pull request Mar 15, 2023
Merged
7 tasks
@vicroms vicroms mentioned this pull request Mar 28, 2023
Closed
@BillyONeal BillyONeal mentioned this pull request Apr 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dg0yt dg0yt dg0yt left review comments

@autoantwort autoantwort autoantwort left review comments

@BillyONeal BillyONeal BillyONeal approved these changes

@vicroms vicroms vicroms approved these changes

@ras0219-msft ras0219-msft ras0219-msft approved these changes

@JackBoosY JackBoosY Awaiting requested review from JackBoosY

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@valeriaconde @dg0yt @vicroms @ras0219-msft @BillyONeal @autoantwort @JackBoosY